feat(mcp): add public AI-Q MCP server#319
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds a standalone MCP server with PostgreSQL-backed jobs, Streamable HTTP tools, isolated packaging and CI, Docker/Compose deployment, release validation, workflow failure contracts, and identifier-log redaction. ChangesAI-Q MCP Server
Packaging, deployment, and validation
Existing workflow hardening
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPRuntime
participant JobManager
participant JobStore
participant WorkflowRunner
Client->>MCPRuntime: submit_query(query)
MCPRuntime->>JobManager: submit(query, anonymous)
JobManager->>WorkflowRunner: classify(query)
JobManager->>JobStore: create queued job
JobManager->>WorkflowRunner: run_query(query, conversation_id)
WorkflowRunner-->>JobManager: workflow outcome
JobManager->>JobStore: update terminal state
Client->>MCPRuntime: poll_query(job_id)
MCPRuntime->>JobManager: poll(job_id, anonymous)
JobManager->>JobStore: record_poll(job_id, anonymous)
Client->>MCPRuntime: get_final_report(job_id)
MCPRuntime->>JobManager: get_final_report(job_id, anonymous)
JobManager->>JobStore: get(job_id)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
91-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin
uvversion consistently across jobs.The
testjob'sSetup uvstep has noversion:pin, whiletest-scripts(Line 186) pins0.11.26. Different jobs in the same workflow run can silently resolve differentuvversions, undermining the reproducibility the version pin intest-scriptsis meant to guarantee.As per path instructions,
.github/**changes should be reviewed for "reproducible uv/npm setup."♻️ Proposed fix
- name: Setup uv uses: astral-sh/setup-uv@v4 with: enable-cache: true + version: "0.11.26"Also applies to: 182-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 91 - 95, The Setup uv steps in the workflow are inconsistent because the test job uses astral-sh/setup-uv@v4 without a version pin while the test-scripts job already pins uv, so align both jobs to the same fixed uv version. Update the Setup uv configuration in the workflow so the test and test-scripts jobs use the same version value, keeping the version reference attached to the astral-sh/setup-uv@v4 step.Source: Path instructions
mcp/tests/conftest.py (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid relying on pytest’s importlib namespace setup. This shim is tied to
--import-mode=importlibpackage resolution, which changed in pytest 8.2; if collection behavior shifts or the mode changes,mcpcan be shadowed again. A less order-dependent import path would make this safer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/tests/conftest.py` around lines 1 - 19, The pytest collection shim in conftest.py is too dependent on importlib-mode namespace behavior, so replace the current sys.modules cleanup plus importlib.import_module("mcp.server.fastmcp") approach with a more direct, order-independent way to ensure the installed MCP package is imported. Update the logic around protocol_package and the mcp module handling so it explicitly resolves the dependency package before tests run, avoiding reliance on pytest’s collection-created namespace module. Keep the fix localized to the conftest.py import bootstrap path and preserve the intended fastmcp import without depending on collection order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 68: The CI workflow uses mutable references for the postgres service
image and the upload-artifact actions, so update the workflow to pin those
dependencies to immutable digests/SHA references. In the workflow definition,
replace the postgres service reference and both uses of actions/upload-artifact
with digest-pinned versions, keeping the existing job behavior unchanged while
making the references reproducible and safer to review.
In `@mcp/src/aiq_mcp/jobs.py`:
- Around line 344-349: The `_heartbeat_job` loop should not die on a transient
`self._store.heartbeat(job_id, self._runner_id)` failure; wrap the heartbeat
call in exception handling similar to `_reconcile_jobs_periodically` so the task
keeps running and logs the error. Add a broad catch inside `_heartbeat_job` with
a retry-after-sleep behavior, and make sure the `JobRunner` cleanup path in
`_run_job` still discards the job from `_FAILURE_HANDLER` even if the heartbeat
task ends unexpectedly.
In `@mcp/src/aiq_mcp/server.py`:
- Around line 331-394: `submit_query` currently accepts unlimited anonymous
requests with an unbounded `query` string, so add abuse controls around this
public entrypoint. Enforce a maximum query length before calling
`_get_jobs().submit`, and add a per-IP or global submission rate limit for the
unauthenticated flow, preferably in the MCP server stack (for example via
Starlette middleware or upstream proxy/ingress). Use `submit_query`,
`_get_jobs`, and the anonymous-principal path as the key locations to update.
In `@mcp/tests/test_client_session_integration.py`:
- Around line 416-434: The skip/warning text in the phase6_postgres_url fixture
is embedding the raw exception string, which can leak DSN or credential-related
details. Update the exception handling around _ensure_database and _reset_schema
to use only the exception type name from asyncpg.PostgresError/OSError instead
of interpolating exc directly, while keeping the existing pytest.skip and
warnings.warn behavior and the same fixture flow.
In `@src/aiq_agent/common/logging_utils.py`:
- Around line 9-12: The log_identifier_ref helper currently uses plain SHA-256,
which is deterministic but easier to brute-force for low-entropy identifiers.
Update log_identifier_ref in logging_utils to use a keyed hash such as HMAC with
a server-side secret while preserving the same stable output contract and
sha256:-prefixed 12-character hex format. Keep the existing test expectations in
mind: deterministic for the same input, does not reveal the original identifier,
and still returns exactly 12 hex characters after the prefix.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 91-95: The Setup uv steps in the workflow are inconsistent because
the test job uses astral-sh/setup-uv@v4 without a version pin while the
test-scripts job already pins uv, so align both jobs to the same fixed uv
version. Update the Setup uv configuration in the workflow so the test and
test-scripts jobs use the same version value, keeping the version reference
attached to the astral-sh/setup-uv@v4 step.
In `@mcp/tests/conftest.py`:
- Around line 1-19: The pytest collection shim in conftest.py is too dependent
on importlib-mode namespace behavior, so replace the current sys.modules cleanup
plus importlib.import_module("mcp.server.fastmcp") approach with a more direct,
order-independent way to ensure the installed MCP package is imported. Update
the logic around protocol_package and the mcp module handling so it explicitly
resolves the dependency package before tests run, avoiding reliance on pytest’s
collection-created namespace module. Keep the fix localized to the conftest.py
import bootstrap path and preserve the intended fastmcp import without depending
on collection order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: cbcc9fa2-3a60-49b5-a035-04b70d24b653
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
.dockerignore.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineLICENSE-THIRD-PARTYREADME.mdconfigs/config_mcp.ymldeploy/compose/docker-compose.mcp.yamldocs/source/index.mddocs/source/integration/index.mddocs/source/integration/mcp-server.mdmcp/Dockerfilemcp/LICENSEmcp/README.mdmcp/REFERENCE_PARITY.mdmcp/SECURITY.mdmcp/deploy/README.mdmcp/deploy/init-mcp-db.sqlmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/scripts/check_runtime_dependencies.pymcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/__init__.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/src/aiq_mcp/db_url.pymcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/conftest.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_db_url.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_imports.pymcp/tests/test_job_store.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_release_checks.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_server_runtime.pymcp/tests/test_workflow_runner.pypyproject.tomlsrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/common/logging_utils.pysrc/aiq_agent/knowledge/summary_store.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/common/test_logging_utils.pytests/knowledge_layer_tests/test_summary_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Script Validation
- GitHub Check: Pytest and Coverage
⚠️ CI failures not shown inline (2)
GitHub Actions: Skills Eval / Run Harbor skill eval: fix(mcp): upgrade NLTK security baseline
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
GitHub Actions: Skills Eval / 0_Run Harbor skill eval.txt: fix(mcp): upgrade NLTK security baseline
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
🧰 Additional context used
📓 Path-based instructions (11)
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
mcp/deploy/README.mddocs/source/integration/index.mdmcp/src/aiq_mcp/__init__.pytests/aiq_agent/common/test_logging_utils.pymcp/README.mdtests/aiq_agent/agents/chat_researcher/test_register_helpers.pyLICENSE-THIRD-PARTYmcp/LICENSEmcp/REFERENCE_PARITY.mdmcp/tests/conftest.pydocs/source/index.mdmcp/tests/test_db_url.pymcp/src/aiq_mcp/db_url.pymcp/tests/test_job_store.pyREADME.mdmcp/scripts/check_runtime_dependencies.pymcp/tests/test_runtime_dependencies.pymcp/pyproject.tomlsrc/aiq_agent/common/logging_utils.pymcp/tests/test_imports.pymcp/tests/test_dependency_compatibility.pymcp/SECURITY.mdmcp/Dockerfilemcp/tests/test_workflow_runner.pymcp/scripts/protocol_smoke.pytests/knowledge_layer_tests/test_summary_store.pymcp/tests/test_deployment_assets.pymcp/deploy/init-mcp-db.sqlsrc/aiq_agent/agents/chat_researcher/register.pyconfigs/config_mcp.ymldocs/source/integration/mcp-server.mdmcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_client_session_integration.pypyproject.tomlmcp/src/aiq_mcp/checkpoint_todos.pymcp/scripts/check_license_inventory.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/jobs.pydeploy/compose/docker-compose.mcp.yamlmcp/tests/test_jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/integration/index.mddocs/source/index.mddocs/source/integration/mcp-server.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/integration/index.mddocs/source/index.mdREADME.mddocs/source/integration/mcp-server.md
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
mcp/src/aiq_mcp/__init__.pytests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/src/aiq_mcp/db_url.pymcp/tests/test_job_store.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_runtime_dependencies.pysrc/aiq_agent/common/logging_utils.pymcp/tests/test_imports.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_workflow_runner.pymcp/scripts/protocol_smoke.pytests/knowledge_layer_tests/test_summary_store.pymcp/tests/test_deployment_assets.pysrc/aiq_agent/agents/chat_researcher/register.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_client_session_integration.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/scripts/check_license_inventory.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
.pre-commit-config.yamlpyproject.toml.github/workflows/ci.yml
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/tests/test_job_store.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_imports.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_workflow_runner.pytests/knowledge_layer_tests/test_summary_store.pymcp/tests/test_deployment_assets.pymcp/tests/test_release_checks.pymcp/tests/test_client_session_integration.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/common/logging_utils.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/register.py
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
configs/config_mcp.ymldeploy/compose/docker-compose.mcp.yaml
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
src/aiq_agent/knowledge/summary_store.py
**/*config*.py
📄 CodeRabbit inference engine (AGENTS.md)
Config schemas must inherit from FunctionBaseConfig and YAML _type names must come from the registered config class
Files:
mcp/tests/test_config_and_packaging.py
🪛 ast-grep (0.44.1)
mcp/scripts/check_runtime_dependencies.py
[info] 89-89: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/scripts/protocol_smoke.py
[info] 137-137: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/scripts/check_license_inventory.py
[info] 171-171: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload, sort_keys=True, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 418-418: use jsonify instead of json.dumps for JSON output
Context: json.dumps(inventory, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 419-419: use jsonify instead of json.dumps for JSON output
Context: json.dumps(validation, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/src/aiq_mcp/server.py
[warning] 66-66: Do not make http calls without encryption
Context: "http://[::1]:*"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 67-67: Do not make http calls without encryption
Context: "http://0.0.0.0:*"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
mcp/tests/test_server_runtime.py
[warning] 479-479: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 546-546: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 559-559: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 566-566: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 628-628: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 640-640: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 645-645: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 661-661: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 695-695: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 710-710: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 721-721: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://untrusted.example"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 933-933: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 721-721: Do not make http calls without encryption
Context: "http://untrusted.example"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[info] 400-400: use jsonify instead of json.dumps for JSON output
Context: json.dumps(schema_contract, sort_keys=True, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Checkov (3.3.2)
mcp/Dockerfile
[low] 10-10: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
🪛 GitHub Actions: Skills Eval / Run Harbor skill eval
mcp/Dockerfile
[error] 79-79: Docker build failed at step [builder 14/17]: uv sync --frozen --extra s3. Error: "Failed to determine installation plan". Caused by: "Distribution not found at: file:///app/mcp". Exit code: 2.
🪛 Hadolint (2.14.0)
mcp/Dockerfile
[warning] 16-16: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[warning] 65-65: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
🪛 OpenGrep (1.23.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 121-137: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 142-153: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 157-168: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 192-195: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 239-239: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 244-255: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 259-259: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 260-269: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 270-288: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 289-291: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 292-294: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 295-295: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 296-298: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 299-301: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 302-310: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 311-319: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_client_session_integration.py
[ERROR] 491-491: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_checkpoint_todos.py
[ERROR] 426-426: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_postgres_job_store.py
[ERROR] 484-484: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 SQLFluff (4.2.2)
mcp/deploy/init-mcp-db.sql
[error] 43-43: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 44-44: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 45-45: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 46-46: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
🪛 zizmor (1.26.1)
.github/workflows/ci.yml
[error] 68-68: unpinned image references (unpinned-images): container image is not pinned to a SHA256 hash
(unpinned-images)
[error] 263-263: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 292-292: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (59)
src/aiq_agent/agents/chat_researcher/register.py (1)
34-34: LGTM!Also applies to: 61-65, 561-574, 618-633, 645-648
src/aiq_agent/knowledge/summary_store.py (1)
30-31: LGTM!Also applies to: 108-108, 135-135, 153-153, 264-270, 286-290, 309-313, 328-334, 347-353, 365-365
tests/aiq_agent/agents/chat_researcher/test_register_helpers.py (1)
18-19: LGTM!Also applies to: 28-40
tests/aiq_agent/common/test_logging_utils.py (1)
1-20: LGTM!tests/knowledge_layer_tests/test_summary_store.py (1)
25-32: LGTM!Also applies to: 218-232
.secrets.baseline (1)
136-136: LGTM!Also applies to: 354-358
mcp/tests/test_deployment_assets.py (2)
91-98: Hash-based schema assertion is reasonably backed by explicit content checks.The SHA-256 comparison against
_EXPECTED_SQL_HASHcould in isolation be "fixed" by simply recomputing the hash after any change, but lines 95-97 also assert specific structural content (mcp_jobstable, index name, migration version rows), which mitigates the rubber-stamp risk. No action needed.
1-155: LGTM!mcp/Dockerfile (2)
10-96: LGTM!
47-58: 🩺 Stability & AvailabilityWrong Dockerfile: the reported
uv sync --frozen --extra s3failure is fromdeploy/Dockerfile:79, notmcp/Dockerfile;mcp/Dockerfiledoesn’t pass--extra s3, and line 79 there isENV HOME=/home/aiq.> Likely an incorrect or invalid review comment.deploy/compose/docker-compose.mcp.yaml (3)
53-53: 🎯 Functional Correctness | ⚡ Quick winInconsistent shell parameter-expansion operator for
AIQ_MCP_CORS_ORIGINS.Every other overridable variable in this service uses
${VAR:-default}(unset-or-empty falls back to default), but this one uses${VAR-default}(only unset falls back; an explicitly empty value is kept as-is). This is inconsistent with the pattern used forAIQ_MCP_PATH,AIQ_MCP_WORKERS,AIQ_MCP_LOG_LEVEL,AIQ_MCP_ALLOWED_HOSTS, andAIQ_MCP_ALLOWED_ORIGINSon the surrounding lines. If this asymmetry is intentional (e.g., to let users explicitly disable CORS by setting an empty string), it should be commented; otherwise it looks like a typo.🔧 Proposed fix
- AIQ_MCP_CORS_ORIGINS: "${AIQ_MCP_CORS_ORIGINS-http://localhost:6274}" + AIQ_MCP_CORS_ORIGINS: "${AIQ_MCP_CORS_ORIGINS:-http://localhost:6274}"
17-22: 🔒 Security & Privacy | ⚡ Quick winPostgres password/checkpoint DB URL has no override mechanism.
Every other configurable value in the
aiq-mcpservice uses the${VAR:-default}pattern so operators can override without editing the file, butPOSTGRES_PASSWORDand the embedded password inAIQ_CHECKPOINT_DBare hardcoded literals. The comment on lines 18-20 explains why a raw password isn't simply interpolated (percent-encoding of reserved URI characters), which is a fair constraint, but it still means anyone reusing this compose file beyond pure loopback-only local testing must hand-edit the YAML to change the credential rather than setting an env var. Since this is adeploy/**config, consider at least parameterizing the default (e.g.,${AIQ_MCP_POSTGRES_PASSWORD:-local_mcp_password}on both lines) so the override path exists even if percent-encoding correctness is the caller's responsibility.As per path instructions, deployment/config changes should be reviewed for "secret separation, safe defaults, local-vs-production behavior" and should flag "committed credentials."
Also applies to: 44-46
Source: Path instructions
1-16: LGTM!Also applies to: 23-43, 47-91
mcp/pyproject.toml (2)
27-38: Version ranges verified as currently valid.Checked
starlette>=1.0,<2anduvicorn>=0.40,<1against current PyPI releases (Starlette is at 1.3.x, Uvicorn is at 0.50.x as of mid-2026), so both ranges resolve correctly and don't exceed real published versions. The exact pin onmcp==1.27.2is also a safe choice given the upstream SDK's own guidance to add a<2upper bound ahead of the v2.0.0 stable release targeted for 2026-07-28.
1-53: LGTM!mcp/src/aiq_mcp/__init__.py (1)
1-7: LGTM!mcp/tests/test_config_and_packaging.py (1)
1-139: LGTM!mcp/tests/test_imports.py (1)
1-67: LGTM!mcp/deploy/init-mcp-db.sql (1)
18-54: LGTM!mcp/deploy/README.md (1)
1-12: LGTM!mcp/scripts/check_license_inventory.py (1)
297-397: LGTM!mcp/tests/test_release_checks.py (1)
1-204: LGTM!mcp/REFERENCE_PARITY.md (1)
1-85: LGTM!LICENSE-THIRD-PARTY (1)
8-11: LGTM!.dockerignore (1)
64-71: 🚀 Performance & ScalabilityKeep
frontends/benchmarks/in the build context.mcp/Dockerfilecopiesfrontends/benchmarks/freshqa/pyproject.tomlandfrontends/benchmarks/deepsearch_qa/pyproject.toml, so re-ignoring that directory would break the build.> Likely an incorrect or invalid review comment.mcp/scripts/check_runtime_dependencies.py (1)
17-96: LGTM!mcp/tests/test_runtime_dependencies.py (1)
1-76: LGTM!mcp/LICENSE (1)
1-202: LGTM!.github/workflows/ci.yml (2)
101-165: LGTM!Also applies to: 208-216, 236-300
217-234: 🩺 Stability & AvailabilityDrop this comment
uv audit --preview-features audit-command,json-outputand--ignore-until-fixedare supported by the pinneduvversion, and the workflow matches the documented invocation.> Likely an incorrect or invalid review comment..pre-commit-config.yaml (1)
70-75: LGTM!pyproject.toml (1)
51-51: LGTM!Also applies to: 101-101, 158-158, 211-212, 214-218, 227-228, 243-243, 257-257
mcp/tests/test_dependency_compatibility.py (2)
33-90: LGTM!
26-30: 🗄️ Data Integrity & Integration
nvidia-natis pinned at the workspace level
pyproject.tomlalready pinsnvidia-nat[langchain,async_endpoints,phoenix,mcp]==1.8.0, so theversion("nvidia-nat")assertion is consistent with the compatibility baseline.> Likely an incorrect or invalid review comment.mcp/scripts/protocol_smoke.py (2)
70-83: 🩺 Stability & Availability | ⚡ Quick winVerify the
ClientSessioncontract before relying on this smoke test.
read_timeout_secondsis passed atimedelta, and the stateless-session assertion depends onget_session_id()semantics. If the pinned MCP SDK expects a numeric timeout or reports a client-side session ID for streamable HTTP, this will fail before it exercises the server contract. Please confirm against the version shipped in this repo and adjust the timeout type or assertion if needed.
1-69: LGTM!Also applies to: 84-143
mcp/SECURITY.md (1)
1-120: LGTM!README.md (1)
41-41: LGTM!Also applies to: 244-244, 305-319
docs/source/index.md (1)
76-76: LGTM!docs/source/integration/index.md (1)
12-12: LGTM!docs/source/integration/mcp-server.md (1)
1-251: LGTM!mcp/README.md (1)
1-167: LGTM!mcp/src/aiq_mcp/job_store.py (2)
121-319: 🔒 Security & PrivacyStatic-analysis SQL-injection hints are false positives.
All f-string interpolations here are table/schema identifiers produced by
_quote_identifier(regex-validated, Line 352-355) — never raw user input. Actual values (job_id, principal, query, etc.) are always passed as$nbind parameters. No injection risk.Source: Linters/SAST tools
1-355: LGTM! Schema migration/versioning, poll/heartbeat reconciliation, and TTL sweep semantics look correct and are exercised by the Postgres integration tests.mcp/src/aiq_mcp/db_url.py (1)
1-21: LGTM!mcp/src/aiq_mcp/checkpoint_todos.py (1)
1-271: LGTM! Fail-soft error handling, redacted logging of the thread capability id, and the latest-todo CTE query all check out against the accompanying tests.mcp/tests/test_db_url.py (1)
1-29: LGTM!mcp/tests/test_job_store.py (1)
1-21: LGTM!mcp/tests/test_checkpoint_todos.py (1)
1-478: LGTM! Solid coverage of the redaction, fail-soft, and namespace-exclusion contracts thatcheckpoint_todos.pyrelies on.mcp/tests/test_postgres_job_store.py (1)
1-509: LGTM! Thorough integration coverage for reconciliation and multi-instance job handoff semantics.configs/config_mcp.yml (1)
1-109: LGTM! Matches the shipped-config contract test exactly, and secrets are correctly deferred to env vars.mcp/src/aiq_mcp/jobs.py (1)
1-343: LGTM!Also applies to: 350-506
mcp/src/aiq_mcp/workflow_runner.py (2)
1-50: LGTM!Also applies to: 90-142
51-89: 🎯 Functional CorrectnessConfirm
load_workflow()doesn’t already own these cachesmcp/src/aiq_mcp/workflow_runner.py:51-89closes checkpointers and pools afterAsyncExitStack.aclose(). If NAT teardown already disposes the same shared_checkpointers/_postgres_poolsentries, this becomes a double-close on shutdown.mcp/tests/test_jobs.py (1)
1-906: LGTM! Thorough golden-contract and redaction coverage; consider adding a case for a heartbeat-write failure mid-run once the_heartbeat_jobfix (see jobs.py) lands.mcp/tests/test_workflow_runner.py (1)
1-170: LGTM!mcp/src/aiq_mcp/server.py (1)
1-330: LGTM!Also applies to: 396-567
mcp/tests/test_server_runtime.py (1)
1-992: LGTM!mcp/tests/test_client_session_integration.py (1)
1-415: LGTM!Also applies to: 435-516
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
mcp/src/aiq_mcp/jobs.py (1)
312-372: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winException during
mark_runningbypasses the new state-guarded failure update.If
mark_runningitself raises (e.g. a transient asyncpg error) rather than returningFalse, execution falls into the genericexcept Exceptionbranch, which marks the job failed usingfrom_states=("running",)andrunner_id=self._runner_id. But the row is stillqueuedwith norunner_idset, so thisupdate()call never matches and silently returnsFalse— the failure is dropped, and the job sitsqueued(visible to pollers as still running) until the stale-job reconciler eventually times it out, rather than failing immediately.🛡️ Proposed fix
async def _run_job(self, job_id: str, query: str) -> None: job_id_token = _current_job_id.set(job_id) job_ref = log_identifier_ref(job_id) heartbeat_task: asyncio.Task | None = None + claimed = False try: claimed = await self._store.mark_running(job_id, self._runner_id) if not claimed: @@ except Exception as exc: # noqa: BLE001 - we want to catch everything for jobs logger.error("Job %s failed (%s)", job_ref, type(exc).__name__) sanitized = _sanitize_error(exc) await self._store.update( job_id, state="failed", error=sanitized, - from_states=("running",), - runner_id=self._runner_id, + from_states=("running",) if claimed else ("queued", "running"), + runner_id=self._runner_id if claimed else None, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/src/aiq_mcp/jobs.py` around lines 312 - 372, The _run_job flow in jobs.py drops failures when _store.mark_running raises before the job row is transitioned to running. Update the exception handling in _run_job so that failures from mark_running are recorded against the queued state instead of always using from_states=("running",) and runner_id=self._runner_id; use the job state/runner context returned by mark_running, or branch the failure update based on whether claiming succeeded. Keep the existing behavior for the later run_query path, but ensure the generic except block can persist a failure even when mark_running never claimed the job.mcp/src/aiq_mcp/server.py (1)
379-454: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winWrap the remaining MCP tool calls before they escape
submit_queryonly sanitizesjobs.submit(...);wait_for_completion,poll_query, andget_final_reportcan still propagate raw internal exceptions through the public MCP error path. Wrap them the same way (or factor a generic helper) so anonymous callers never see backend exception text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/src/aiq_mcp/server.py` around lines 379 - 454, The remaining public MCP tool paths in `submit_query`, `poll_query`, and `get_final_report` still allow raw backend exceptions to escape. Update the `MCP` server methods to sanitize errors consistently by wrapping `jobs.wait_for_completion(...)`, `self._get_jobs().poll(...)`, and `self._get_jobs().get_final_report(...)` in the same public error handling used for `jobs.submit(...)`, or factor a shared helper for `_public_submit_error`-style translation. Ensure anonymous callers only receive sanitized failures and never backend exception text.frontends/ui/src/features/chat/store.ts (1)
2911-2922: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve existing file content when merging artifact updates
artifact.updatefile events emitcontent: undefinedfor durable artifacts, so{ ...prev, ...file }clears earlier text when a later metadata-only update arrives for the same filename. Merge only defined fields infrontends/ui/src/features/chat/store.ts,frontends/ui/src/features/chat/hooks/use-deep-research.ts, andfrontends/ui/src/features/chat/hooks/use-load-job-data.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/ui/src/features/chat/store.ts` around lines 2911 - 2922, The deep research file merge logic is overwriting existing content with undefined fields from later metadata-only updates. Update the merge behavior in addDeepResearchFile, use-deep-research, and use-load-job-data so only defined values from the incoming file are applied, while preserving existing content and other previously stored fields for the same filename. Use the existing identifiers addDeepResearchFile, useDeepResearch, and useLoadJobData to locate the merge points and make the update logic skip undefined properties instead of spreading them blindly.src/aiq_agent/agents/deep_researcher/agent.py (1)
184-202: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a regression test for the
state.filesfallback. The current coverage hitsresult["files"]and the end-to-end/shared/output.mdflow, but not the branch whereresulthas no usable"files"key and_extract_final_markdownmust read the markdown from thefilesargument.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 184 - 202, Add a regression test for the state.files fallback in _extract_final_markdown: current coverage only exercises result["files"] and the end-to-end output path, but not the branch where result has no usable "files" key and the method must use the files argument. Create a focused test around DeepResearcherAgent._extract_final_markdown that passes a result without valid files and a files dict containing /shared/output.md or /output.md, then assert the markdown is returned from that fallback path rather than _salvage_inline_report.Source: Path instructions
mcp/tests/test_jobs.py (1)
67-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
_MemoryJobStoreacross test modules.This class is near-identical to
_MemoryJobStoreinmcp/tests/test_client_session_integration.py. Extracting a shared test double (e.g. into aconftest.pyor a_test_doubles.pyhelper) would avoid the two copies drifting out of sync asJobStore's real contract (guards, TTL, heartbeat semantics) evolves.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/tests/test_jobs.py` around lines 67 - 173, The `_MemoryJobStore` test double is duplicated across test modules and should be shared to prevent drift as `JobStore` behavior changes. Move the common implementation behind a reusable test helper (for example a shared `_test_doubles.py` module or `conftest.py` fixture) and update the tests to import/use that single `_MemoryJobStore` definition, keeping the methods like `create`, `mark_running`, `heartbeat`, `update`, and `mark_stale_running_failed` in one place.frontends/ui/src/features/layout/components/FileCard.tsx (1)
96-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale
aria-expanded/aria-controlson non-expandable (artifact-only) cards.The header
<Button>still exposesaria-expandedandaria-controls={file-content-${file.id}}even whenfile.contentis absent (now a common case for artifact-only cards with justcontentUrl/mimeType). The click handler is a no-op in that case, but the DOM still advertises an expandable/collapsible control to assistive tech, and the referenced region never renders.As per path instructions, UI changes should be reviewed for "accessible controls."♿ Proposed fix
<Button kind="tertiary" size="small" onClick={() => file.content && setIsExpanded(!isExpanded)} - aria-expanded={isExpanded} - aria-controls={`file-content-${file.id}`} + aria-expanded={file.content ? isExpanded : undefined} + aria-controls={file.content ? `file-content-${file.id}` : undefined} className="w-full justify-start text-left p-0" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/ui/src/features/layout/components/FileCard.tsx` around lines 96 - 155, In FileCard, the header Button still advertises expand/collapse behavior for artifact-only files that have no content, so make the accessibility props conditional. Update the FileCard button/expansion logic so aria-expanded, aria-controls, and the click toggle are only applied when file.content exists, and ensure the expandable region and ChevronDown icon are only rendered for expandable cards. Use the existing FileCard and file.content checks to keep non-expandable cards from exposing stale accessible controls.Source: Path instructions
mcp/tests/test_deployment_assets.py (1)
136-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore key/cert exclusions in the deployment context guardrail
mcp/tests/test_deployment_assets.py:136-145no longer asserts**/*.key,**/*.crt, or**/*.pem, andmcp/.dockerignoreonly excludes**/certs/. That leaves loose private key/cert files unguarded in the Docker build context.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/tests/test_deployment_assets.py` around lines 136 - 145, The deployment context test is missing coverage for private key and certificate file exclusions, so update test_deployment_context_excludes_env_files_and_includes_package_readmes to also assert the Docker ignore rules for **/*.key, **/*.crt, and **/*.pem. Then adjust mcp/.dockerignore so these file patterns are explicitly excluded, using the existing deployment guardrail entries alongside the README allowlist.scripts/start_e2e.sh (1)
89-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the CLI
--portafter sourcingdeploy/.env.check_envloadsdeploy/.envafter argument parsing, so any localPORTexport will override the flag beforeBACKEND_URL,nat serve --port, and the health check use it. The trackeddeploy/.env.exampleleavesPORTunset, but a user.envcan still set it, so re-applying the CLI value aftersourcekeeps the explicit flag authoritative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/start_e2e.sh` around lines 89 - 104, After sourcing deploy/.env in start_e2e.sh, the CLI --port value can be overwritten by a PORT entry from the env file, which then affects BACKEND_URL, NEXT_PUBLIC_BACKEND_URL, nat serve --port, and the health check. Re-apply the parsed port value after the source block in check_env/startup flow so the explicit command-line flag remains authoritative, and ensure the PORT variable used by the existing BACKEND_URL export reflects that restored CLI value.
♻️ Duplicate comments (2)
frontends/ui/src/features/chat/hooks/use-load-job-data.ts (1)
577-582: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSame content-drop defect as
use-deep-research.tsandstore.ts'saddDeepResearchFile.
prev ? { ...prev, ...file } : fileoverwritesprev.contentwithundefinedwhen a laterartifact.updateevent for the same filename lacks inline content, contradicting the comment's own intent.🐛 Proposed fix
onFileUpdate: (file) => { // Merge like the live store: a later metadata-only event must not drop // content from an earlier event for the same filename during replay. const prev = buffer.files.get(file.filename) - buffer.files.set(file.filename, prev ? { ...prev, ...file } : file) + const definedUpdates = Object.fromEntries( + Object.entries(file).filter(([, value]) => value !== undefined) + ) as typeof file + buffer.files.set(file.filename, prev ? { ...prev, ...definedUpdates } : file) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/ui/src/features/chat/hooks/use-load-job-data.ts` around lines 577 - 582, The file replay merge in onFileUpdate is still dropping previously captured content when a later artifact.update arrives without inline content. Update the merge logic in use-load-job-data to preserve prev.content unless the incoming file actually provides content, matching the intended behavior already used in use-deep-research.ts and store.ts’s addDeepResearchFile. Keep the rest of the file metadata merged as before, but avoid overwriting existing content with undefined when only metadata changes.frontends/ui/src/features/chat/hooks/use-deep-research.ts (1)
490-505: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBuffer merge doesn't actually preserve content — same root cause as
store.ts'saddDeepResearchFile.
{ ...prev, ...file }copiesfile.contenteven when it's explicitlyundefined(which every non-inline artifact.update event carries perdeep-research-client.ts), overwritingprev.contentfrom an earlier replayed event. This defeats the comment's own stated goal during replay/reconnect.🐛 Proposed fix — filter undefined keys before merging
onFileUpdate: (file) => { if (buf.active) { // Merge like the live store (addDeepResearchFile): a later metadata-only // event must not drop content from an earlier event for the same filename. const prev = buf.files.get(file.filename) - buf.files.set(file.filename, prev ? { ...prev, ...file } : file) + const definedUpdates = Object.fromEntries( + Object.entries(file).filter(([, value]) => value !== undefined) + ) as typeof file + buf.files.set(file.filename, prev ? { ...prev, ...definedUpdates } : file) return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/ui/src/features/chat/hooks/use-deep-research.ts` around lines 490 - 505, The replay buffer merge in onFileUpdate still drops previously captured content because spreading prev and file copies undefined content from later metadata-only events over earlier data. Update the merge logic used in use-deep-research.ts so undefined fields are filtered out before combining, matching the behavior of addDeepResearchFile in store.ts and preserving existing content during reconnect/replay. Keep the fix localized to the buffered path inside onFileUpdate, ensuring only defined properties from file overwrite prev.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 883-884: The failure path in runner.py is persisting the raw
exception text via job_store.update_status in the job runner, which can leak
secrets or internal hostnames. Update the exception handling around the job
status failure update to store only a sanitized error summary or the exception
class name, following the same pattern used by _teardown_sandbox, and keep the
rest of the context in logs without exposing the raw message.
- Around line 859-866: The cancellation cleanup in runner.py is too narrowly
guarded, so driver-specific or transient job-store errors can still escape and
prevent the job from reaching INTERRUPTED. Update the exception handling around
job_store.get_job and job_store.update_status in the cancellation branch of the
runner logic to catch and swallow/log a broader set of job-store failures, using
the existing job_store and JobStatus.INTERRUPTED flow so cancellation never
leaves the job stranded.
In `@frontends/ui/src/adapters/api/deep-research-client.spec.ts`:
- Around line 5-27: Add a regression test around
createDeepResearchClient/getJobStatus using FakeEventSource that emits an
initial file-artifact event with content and then a later metadata-only
artifact.update for the same filename; assert the stored result keeps the
original content instead of being replaced. This should exercise the merge
behavior that was broken in addDeepResearchFile and the hook buffer paths, so
the test should verify the filename entry retains both metadata and previously
captured content after the second update.
In `@skills/aiq-research/skill-card.md`:
- Around line 33-34: The markdown under the Skill Output section is missing the
required blank line after the heading, causing the MD022 lint failure. Update
the content around the “## Skill Output:” heading in skill-card.md so that there
is an empty line before the “Output Type(s):” line, keeping the existing wording
intact.
---
Outside diff comments:
In `@frontends/ui/src/features/chat/store.ts`:
- Around line 2911-2922: The deep research file merge logic is overwriting
existing content with undefined fields from later metadata-only updates. Update
the merge behavior in addDeepResearchFile, use-deep-research, and
use-load-job-data so only defined values from the incoming file are applied,
while preserving existing content and other previously stored fields for the
same filename. Use the existing identifiers addDeepResearchFile,
useDeepResearch, and useLoadJobData to locate the merge points and make the
update logic skip undefined properties instead of spreading them blindly.
In `@frontends/ui/src/features/layout/components/FileCard.tsx`:
- Around line 96-155: In FileCard, the header Button still advertises
expand/collapse behavior for artifact-only files that have no content, so make
the accessibility props conditional. Update the FileCard button/expansion logic
so aria-expanded, aria-controls, and the click toggle are only applied when
file.content exists, and ensure the expandable region and ChevronDown icon are
only rendered for expandable cards. Use the existing FileCard and file.content
checks to keep non-expandable cards from exposing stale accessible controls.
In `@mcp/src/aiq_mcp/jobs.py`:
- Around line 312-372: The _run_job flow in jobs.py drops failures when
_store.mark_running raises before the job row is transitioned to running. Update
the exception handling in _run_job so that failures from mark_running are
recorded against the queued state instead of always using
from_states=("running",) and runner_id=self._runner_id; use the job state/runner
context returned by mark_running, or branch the failure update based on whether
claiming succeeded. Keep the existing behavior for the later run_query path, but
ensure the generic except block can persist a failure even when mark_running
never claimed the job.
In `@mcp/src/aiq_mcp/server.py`:
- Around line 379-454: The remaining public MCP tool paths in `submit_query`,
`poll_query`, and `get_final_report` still allow raw backend exceptions to
escape. Update the `MCP` server methods to sanitize errors consistently by
wrapping `jobs.wait_for_completion(...)`, `self._get_jobs().poll(...)`, and
`self._get_jobs().get_final_report(...)` in the same public error handling used
for `jobs.submit(...)`, or factor a shared helper for
`_public_submit_error`-style translation. Ensure anonymous callers only receive
sanitized failures and never backend exception text.
In `@mcp/tests/test_deployment_assets.py`:
- Around line 136-145: The deployment context test is missing coverage for
private key and certificate file exclusions, so update
test_deployment_context_excludes_env_files_and_includes_package_readmes to also
assert the Docker ignore rules for **/*.key, **/*.crt, and **/*.pem. Then adjust
mcp/.dockerignore so these file patterns are explicitly excluded, using the
existing deployment guardrail entries alongside the README allowlist.
In `@mcp/tests/test_jobs.py`:
- Around line 67-173: The `_MemoryJobStore` test double is duplicated across
test modules and should be shared to prevent drift as `JobStore` behavior
changes. Move the common implementation behind a reusable test helper (for
example a shared `_test_doubles.py` module or `conftest.py` fixture) and update
the tests to import/use that single `_MemoryJobStore` definition, keeping the
methods like `create`, `mark_running`, `heartbeat`, `update`, and
`mark_stale_running_failed` in one place.
In `@scripts/start_e2e.sh`:
- Around line 89-104: After sourcing deploy/.env in start_e2e.sh, the CLI --port
value can be overwritten by a PORT entry from the env file, which then affects
BACKEND_URL, NEXT_PUBLIC_BACKEND_URL, nat serve --port, and the health check.
Re-apply the parsed port value after the source block in check_env/startup flow
so the explicit command-line flag remains authoritative, and ensure the PORT
variable used by the existing BACKEND_URL export reflects that restored CLI
value.
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 184-202: Add a regression test for the state.files fallback in
_extract_final_markdown: current coverage only exercises result["files"] and the
end-to-end output path, but not the branch where result has no usable "files"
key and the method must use the files argument. Create a focused test around
DeepResearcherAgent._extract_final_markdown that passes a result without valid
files and a files dict containing /shared/output.md or /output.md, then assert
the markdown is returned from that fallback path rather than
_salvage_inline_report.
---
Duplicate comments:
In `@frontends/ui/src/features/chat/hooks/use-deep-research.ts`:
- Around line 490-505: The replay buffer merge in onFileUpdate still drops
previously captured content because spreading prev and file copies undefined
content from later metadata-only events over earlier data. Update the merge
logic used in use-deep-research.ts so undefined fields are filtered out before
combining, matching the behavior of addDeepResearchFile in store.ts and
preserving existing content during reconnect/replay. Keep the fix localized to
the buffered path inside onFileUpdate, ensuring only defined properties from
file overwrite prev.
In `@frontends/ui/src/features/chat/hooks/use-load-job-data.ts`:
- Around line 577-582: The file replay merge in onFileUpdate is still dropping
previously captured content when a later artifact.update arrives without inline
content. Update the merge logic in use-load-job-data to preserve prev.content
unless the incoming file actually provides content, matching the intended
behavior already used in use-deep-research.ts and store.ts’s
addDeepResearchFile. Keep the rest of the file metadata merged as before, but
avoid overwriting existing content with undefined when only metadata changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d6f875fe-2cc7-4d87-ac22-6dbd26ae5a84
⛔ Files ignored due to path filters (1)
skills/aiq-research/skill.oms.sigis excluded by!skills/**/skill.oms.sig
📒 Files selected for processing (59)
.dockerignoredeploy/helm/deployment-k8s/Chart.yamldeploy/helm/deployment-k8s/charts/aiq-0.0.4.tgzdeploy/helm/deployment-k8s/charts/aiq-0.0.5.tgzdeploy/helm/helm-charts-k8s/aiq/Chart.yamldeploy/helm/helm-charts-k8s/aiq/templates/_helpers.tpldeploy/helm/helm-charts-k8s/aiq/templates/namespace.yamldeploy/helm/helm-charts-k8s/aiq/values.yamldocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/jobs/telemetry.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/shared/utils/artifact-url.spec.tsfrontends/ui/src/shared/utils/artifact-url.tsmcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/tests/test_client_session_integration.pymcp/tests/test_deployment_assets.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.pyscripts/start_e2e.shskills/aiq-research/BENCHMARK.mdskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pyskills/aiq-research/skill-card.mdsrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/tools/research.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.py
💤 Files with no reviewable changes (1)
- .dockerignore
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: UI Unit Tests
- GitHub Check: Script Validation
- GitHub Check: Pytest and Coverage
⚠️ CI failures not shown inline (3)
GitHub Actions: Skills Eval / Run Harbor skill eval: fix(mcp): harden public job failure handling
Conclusion: failure
##[group]Run set -a
�[36;1mset -a�[0m
�[36;1m# shellcheck disable=SC1090�[0m
�[36;1msource "$RUNNER_ENV_FILE"�[0m
�[36;1mset +a�[0m
�[36;1m# Mirror local recipe: build from source, tag with run id so�[0m
�[36;1m# concurrent runs (if any) don't collide.�[0m
�[36;1mexport BACKEND_IMAGE="aiq-agent:ci-28987905264"�[0m
�[36;1mdocker compose --env-file ../.env -f docker-compose.yaml up -d --build aiq-agent postgres�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
RUNNER_ENV_FILE: /home/ubuntu/aiq-eval/.env
pythonLocation: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
PKG_CONFIG_PATH: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64/lib/pkgconfig
Python_ROOT_DIR: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
Python2_ROOT_DIR: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
Python3_ROOT_DIR: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
LD_LIBRARY_PATH: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64/lib
UV_CACHE_DIR: /home/ubuntu/actions-runner/_work/_temp/setup-uv-cache
##[endgroup]
Image aiq-agent:ci-28987905264 Building
`#1` [internal] load local bake definitions
`#1` reading from stdin 583B done
`#1` DONE 0.0s
`#2` [internal] load build definition from Dockerfile
`#2` transferring dockerfile: 5.60kB done
`#2` DONE 0.0s
`#3` [internal] load metadata for nvcr.io/nvidia/base/ubuntu:noble-20260217
`#3` ...
`#4` [internal] load metadata for nvcr.io/nvidia/distroless/python:3.13-v4.0.5
`#4` DONE 0.3s
`#3` [internal] load metadata for nvcr.io/nvidia/base/ubuntu:noble-20260217
`#3` DONE 0.4s
`#5` [internal] load .dockerignore
`#5` transferring context: 1.68kB done
`#5` DONE 0.0s
`#6` [dev 1/4] FROM nvcr.io/nvidia/distroless/python:3.13-v4.0.5@sha256:***REDACTED***
`#6` resolve nvcr.io/nvidia/distroless/python:3.13-v4.0.5@sha256:***REDACTED*** 0.0s done
`#6` DONE 0.0s
`#7` [builder 1/17] FROM nvcr.io/nvidia/base/ubuntu:noble-20260217@sha256:***REDACTED***
`#7` resolve ...
GitHub Actions: Skills Eval / Run Harbor skill eval: fix(mcp): harden public job failure handling
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
GitHub Actions: Skills Eval / 0_Run Harbor skill eval.txt: fix(mcp): harden public job failure handling
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
🧰 Additional context used
📓 Path-based instructions (14)
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
deploy/helm/helm-charts-k8s/aiq/Chart.yamldeploy/helm/helm-charts-k8s/aiq/templates/namespace.yamlfrontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsdeploy/helm/helm-charts-k8s/aiq/templates/_helpers.tplskills/aiq-research/BENCHMARK.mddeploy/helm/helm-charts-k8s/aiq/values.yamlfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxdocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/README.mdfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/base.pytests/aiq_agent/agents/deep_researcher/test_factory.pyfrontends/ui/src/shared/components/MarkdownRenderer/index.tsdeploy/helm/deployment-k8s/Chart.yamlfrontends/aiq_api/src/aiq_api/jobs/telemetry.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tstests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pyskills/aiq-research/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pyfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tssrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pyscripts/start_e2e.shfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tstests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pyskills/aiq-research/skill-card.mdsrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pyskills/aiq-research/scripts/aiq.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pyfrontends/ui/src/features/layout/components/FileCard.tsxtests/aiq_agent/agents/deep_researcher/test_agent.pymcp/tests/test_deployment_assets.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.pyfrontends/ui/src/adapters/api/deep-research-client.tstests/aiq_agent/agents/deep_researcher/test_custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdmcp/tests/test_client_session_integration.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_server_runtime.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.py
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/helm/helm-charts-k8s/aiq/Chart.yamldeploy/helm/helm-charts-k8s/aiq/templates/namespace.yamldeploy/helm/helm-charts-k8s/aiq/templates/_helpers.tpldeploy/helm/helm-charts-k8s/aiq/values.yamldeploy/helm/deployment-k8s/Chart.yaml
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/adapters/api/deep-research-client.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/adapters/api/deep-research-client.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/adapters/api/deep-research-client.ts
skills/aiq-research/**
⚙️ CodeRabbit configuration file
skills/aiq-research/**: ---
name: aiq-research
description: |
Use when asked to run deep research or AI-Q research through a reachable NVIDIA AI-Q Blueprint backend.
license: Apache-2.0
permissions:
env:
- AIQ_SERVER_URL
network:
- http://localhost:8000
compatibility: |
Designed for Claude Code, OpenCode, Codex, and Agent Skills-compatible tools. Requires Python 3.11+ and network
access to a running local AI-Q Blueprint server athttp://localhost:8000by default. Non-local backends must be
explicitly trusted by the user and granted by the host tool outside this public skill.
metadata:
version: "2.1.0"
author: "NVIDIA AI-Q Blueprint Team aiq-blueprint@nvidia.com"
github-url: "https://github.com/NVIDIA-AI-Blueprints/aiq"
tags:
- nvidia
- aiq
- blueprint
- deep-research
- research-agents
- agent-skills
languages:
- python
- bash
domain: "research-agents"
allowed-tools: Read BashAIQ Research Skill
Purpose
Use this skill to call a locally running NVIDIA AI-Q Blueprint server through the helper script at
scripts/aiq.py.Use this skill for research-shaped requests, including:
- "deep research on ..."
- "AIQ research ..."
- "research ..."
- "use AI-Q to answer ..."
- "ask AI-Q about ..."
Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests. Those
belong toaiq-deploy.Prerequisites
Users need:
- Python 3.11+ available as
python3.- A reachable local or self-hosted AI-Q Blueprint backend.
AIQ_SERVER_URLset when the backend is not running athttp://localhost:8000; non-local values must be trusted by
the user before any query is sent.- A backend configured with authentication disabled for this public helper, or a separate authenticated AI-Q skill for
authenticated environments.- Network access from the local machine to the AI-Q backend URL.
- Credentials configured in the backend environment, not in this skill. Thi...
Files:
skills/aiq-research/BENCHMARK.mdskills/aiq-research/SKILL.mdskills/aiq-research/skill-card.mdskills/aiq-research/scripts/aiq.py
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/BENCHMARK.mdskills/aiq-research/SKILL.mdskills/aiq-research/skill-card.mdskills/aiq-research/scripts/aiq.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/architecture/agents/sandbox.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/architecture/agents/sandbox.md
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
src/aiq_agent/agents/deep_researcher/sandbox/base.pytests/aiq_agent/agents/deep_researcher/test_factory.pyfrontends/aiq_api/src/aiq_api/jobs/telemetry.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pyskills/aiq-research/scripts/aiq.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_agent.pymcp/tests/test_deployment_assets.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pymcp/tests/test_client_session_integration.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_server_runtime.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.md
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_agent.pymcp/tests/test_deployment_assets.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pymcp/tests/test_client_session_integration.pymcp/tests/test_server_runtime.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/jobs/telemetry.pyfrontends/aiq_api/src/aiq_api/jobs/runner.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Use this skill only for research-shaped requests that need a reachable NVIDIA AI-Q Blueprint backend via `scripts/aiq.py`; do not use it for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests, which belong to `aiq-deploy`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Before sending any user query to AI-Q, resolve the backend URL, run `health`, state the exact endpoint to the user, and only proceed with non-local URLs after explicit trust confirmation in the conversation.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: If no backend is reachable, ask for an AI-Q backend URL or hand off to `aiq-deploy`; if the backend returns `401` or `403`, stop and explain that this public skill does not manage authentication.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Do not send credentials, cookies, bearer tokens, or secret values in AI-Q query text or follow-up query text.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: When AI-Q returns a deep-research job ID, poll asynchronous jobs with `research_poll`, tell the user deep research is running in the background, and do not retry failed jobs automatically.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: If polling is interrupted, resume using `status`, `report`, or `research_poll`; use `status` to inspect job state and artifacts, `report` for finished jobs, and `research_poll` to continue waiting.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Present returned reports with citations and source URLs intact; do not truncate them.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: After presenting a report, answer follow-up questions from the existing report directly when possible; otherwise send a fresh query that carries prior context, and use the same backend/auth/polling flow.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: For a redo, rerun research with a refined query and appropriate agent type, treating it as a new job and restating the target endpoint before sending it.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Respect the skill’s version compatibility rules: the Blueprint major version must match the skill major version, the Blueprint minor version must be equal or greater, and patch version may vary.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: If the backend is reachable but `/chat` or async job routes fail, report that the backend is reachable but not compatible with this public research flow; do not fabricate answers.
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/deep_researcher/test_agent.py
🪛 ast-grep (0.44.1)
tests/deploy/test_helm_deployment_k8s.py
[error] 26-31: Command coming from incoming request
Context: subprocess.run(
["helm", "template", "aiq", str(CHART_PATH), "-n", namespace, *extra_args],
check=True,
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 LanguageTool
skills/aiq-research/skill-card.md
[style] ~12-~12: Consider a different adjective to strengthen your wording.
Context: ... and engineers use this skill to submit deep research queries to a running NVIDIA AI...
(DEEP_PROFOUND)
[style] ~37-~37: Consider a different adjective to strengthen your wording.
Context: ...Output:** [Asynchronous job polling for deep research; artifact download for charts ...
(DEEP_PROFOUND)
🪛 markdownlint-cli2 (0.22.1)
skills/aiq-research/skill-card.md
[warning] 33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 OpenGrep (1.23.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 142-154: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 202-205: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcp/Dockerfile (1)
53-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the inline version-assertion one-liner into the existing check script.
Line 54 hardcodes
version('mcp') == '1.27.2'andversion('nvidia-nat-core') == '1.8.0'inline in a long, hard-to-diff Python one-liner, duplicating information that already lives inmcp/pyproject.toml/mcp/uv.lock. A dedicatedcheck_runtime_dependencies.pyscript already exists one line above (line 53) for this exact purpose — moving the import/version/entry-point assertions there would keep the release "canary" logic testable and reviewable outside the Dockerfile, instead of as an unwrapped shell string.♻️ Proposed consolidation
-RUN /opt/venv/bin/python mcp/scripts/check_runtime_dependencies.py \ - && /opt/venv/bin/python -c "import importlib; from importlib.metadata import entry_points, version; [importlib.import_module(name) for name in ('aiq_mcp', 'aiq_mcp.server', 'aiq_mcp.jobs', 'aiq_mcp.job_store', 'aiq_mcp.workflow_runner', 'aiq_agent.common', 'tavily_web_search', 'knowledge_layer', 'asyncpg')]; assert version('mcp') == '1.27.2'; assert version('nvidia-nat-core') == '1.8.0'; assert any(ep.name == 'tavily_web_search' for ep in entry_points(group='nat.plugins')); print('AI-Q MCP runtime verified')" +RUN /opt/venv/bin/python mcp/scripts/check_runtime_dependencies.pyMove the imports,
version(...) ==assertions, and entry-point check intocheck_runtime_dependencies.pyso version drift shows up as a clean diff in a reviewable Python file rather than a one-line shell string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/Dockerfile` around lines 53 - 54, The Dockerfile’s long Python one-liner duplicates runtime checks that should live in the existing check_runtime_dependencies.py script. Move the import/module validation, version assertions for mcp and nvidia-nat-core, and the nat.plugins entry-point check into check_runtime_dependencies.py, then keep the Dockerfile RUN step limited to invoking that script and the final verification print.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@mcp/Dockerfile`:
- Around line 53-54: The Dockerfile’s long Python one-liner duplicates runtime
checks that should live in the existing check_runtime_dependencies.py script.
Move the import/module validation, version assertions for mcp and
nvidia-nat-core, and the nat.plugins entry-point check into
check_runtime_dependencies.py, then keep the Dockerfile RUN step limited to
invoking that script and the final verification print.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 7ab0f028-d2f3-4e26-b5d2-e4713290cb60
⛔ Files ignored due to path filters (2)
mcp/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.coderabbit.yaml.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineAGENTS.mdCONTRIBUTING.mdLICENSE-THIRD-PARTYREADME.mddeploy/Dockerfiledocs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.mdmcp/Dockerfilemcp/README.mdmcp/SECURITY.mdmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_jobs.pymcp/tests/test_release_checks.pypyproject.tomlscripts/dev.shscripts/setup.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Script Validation
🧰 Additional context used
📓 Path-based instructions (9)
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
LICENSE-THIRD-PARTYdocs/source/contributing/code-style.mdscripts/dev.shmcp/README.mddeploy/DockerfileCONTRIBUTING.mdscripts/setup.shdocs/source/contributing/pr-workflow.mdmcp/pyproject.tomlREADME.mdmcp/Dockerfiledocs/source/contributing/testing.mdAGENTS.mdmcp/tests/test_dependency_compatibility.pydocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.mdmcp/tests/test_deployment_assets.pymcp/SECURITY.mdmcp/tests/test_config_and_packaging.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_jobs.pymcp/scripts/check_license_inventory.pypyproject.toml
docs/source/contributing/**
⚙️ CodeRabbit configuration file
docs/source/contributing/**:Code Organization
High-level layout (refer to Architecture Overview for component roles):
src/aiq_agent/ ├── agents/ # Chat researcher, shallow/deep research, clarifier │ ├── chat_researcher/ # Orchestrator, orchestration node (intent + meta + depth), nodes │ ├── shallow_researcher/ │ ├── deep_researcher/ # See src/aiq_agent/agents/deep_researcher/README.md │ └── clarifier/ ├── common/ # LLM provider, callbacks, prompt utils, data_sources ├── knowledge/ # Schema, factory, base retriever/ingestor, summary store ├── observability/ # OpenTelemetry header redaction exporter ├── auth/ # Auth utilities └── fastapi_extensions/ # API route extensionsConfigs live in
configs/; refer to the Customization guide for configuration options. Frontends:frontends/cli,frontends/aiq_api,frontends/debug,frontends/ui. Benchmarks:frontends/benchmarks/. Data sources and Knowledge Layer:sources/.
docs/source/contributing/**:Code Style
Development Workflow
Helper script:
./scripts/dev.sh help # List commands ./scripts/dev.sh test # Run tests ./scripts/dev.sh format # Format code ./scripts/dev.sh lint # Lint ./scripts/dev.sh pre-commit # Format + checksOtherwise run
pre-commit run --all-files,pytest, and formatters directly (refer to Code Quality below).Code Quality
- Formatting:
ruff(imports/linting) +yapf(PEP 8 base,column_limit=120). The./scripts/dev.sh formatcommand runs both.- Pre-commit: `pre-commit ru...
Files:
docs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/contributing/code-style.mdCONTRIBUTING.mddocs/source/contributing/pr-workflow.mdREADME.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.md
mcp/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For changes under
mcp/, also runuv sync --project mcp --extra devanduv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/README.mdmcp/pyproject.tomlmcp/Dockerfilemcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/SECURITY.mdmcp/tests/test_config_and_packaging.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_jobs.pymcp/scripts/check_license_inventory.py
deploy/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets or environment-specific hostnames in deployment assets; use
deploy/.envfor secrets.
Files:
deploy/Dockerfile
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/Dockerfile
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md
{pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Dependency changes must leave both project locks current: check the root with
uv lock --checkand MCP withuv lock --project mcp --check.
Files:
mcp/pyproject.tomlpyproject.toml
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
mcp/pyproject.toml.pre-commit-config.yaml.github/workflows/ci.ymlpyproject.toml
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: These rules apply to every task in this repository; load the relevant task-specific skill from `.agents/skills/` before starting a workflow it covers.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Stay inside this repository; do not edit adjacent repositories as part of an AI-Q change, and prefer the smallest change scoped to the package you are touching.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Do not commit secrets, tokens, or environment-specific hostnames; use environment variables and `SecretStr`, and resolve API keys at runtime.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Never print or log secret values, including in tool output or error messages.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Missing-secret paths must degrade gracefully (stub or skip) rather than crash or leak.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Respect authenticated data sources: honor `requires_auth`, per-user token pass-through, and backend token validators, and apply owner guardrails before loading protected report or artifact context into an agent.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Do not weaken or bypass `AuthMiddleware`, validators, or auth gating without a prior design discussion.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: For substantial behavior, auth, UI, or architecture changes, open a design discussion before coding rather than landing a large unreviewed change.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Every commit must be signed off with `git commit -s` and include a `Signed-off-by` trailer.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Keep pull requests scoped: avoid unrelated files, accidental generated artifacts, and secrets, and provide validation evidence for the changes made.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Search existing issues and pull requests before opening new work.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Open an issue or discussion before large design changes, public API changes, deployment changes, or contributor workflow changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts in contributions.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Target the `develop` branch unless a maintainer asks you to use a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Make the smallest coherent change and add or update tests for behavior changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Sign off every commit with `git commit -s`; commits without sign-off may be rejected.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Run the relevant local validation before opening the pull request and include the exact output or workflow link in the PR.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Open a pull request into `develop` and fill out the PR template with exact validation evidence.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Address review feedback until required checks and code-owner review pass.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: For deployment changes, run the relevant Helm or compose validation and describe the environment used.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: AI-Q uses push-triggered GitHub Actions; pull requests are mirrored by copy-pr-bot after `/ok to test`, and maintainers can request bot-driven merge with `/merge` when repository rules are satisfied.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: All contributors must sign off their commits to certify DCO compliance and acknowledgment that contributions are public and may be redistributed.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:08.070Z
Learning: Use `./scripts/dev.sh help`, `./scripts/dev.sh test`, `./scripts/dev.sh format`, `./scripts/dev.sh lint`, and `./scripts/dev.sh pre-commit` for the documented development workflow; otherwise run `pre-commit run --all-files`, `pytest`, and the relevant formatters directly.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Create a focused branch from `develop`, make changes, and add or update tests before opening a pull request.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Run the narrowest relevant local checks for your change and record the exact commands in the pull request description.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Open pull requests into `develop` unless a maintainer asks you to target a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Address review feedback until required checks, code-owner review, and review-thread resolution pass.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Use the repository's configured merge/bot workflow when available; otherwise follow the normal protected-branch merge flow.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Sign off all commits with a Developer's Certificate of Origin (DCO) using `git commit -s` so each commit includes a `Signed-off-by` line.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: By contributing, certify compliance with the project's Developer's Certificate of Origin 1.1 statement, including that the contribution is properly licensed and that your sign-off may be retained indefinitely.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: For MCP changes, validate the MCP subproject independently with its own dev environment and test command.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: For UI changes, run the UI project's lint, type-check, test, and build commands from `frontends/ui`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Repository owners, organization members, and collaborators may request additional validation, including NVSkills validation with `/nvskills-ci`.
🪛 SkillSpector (2.3.7)
.agents/skills/aiq-release-qa/SKILL.md
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🔇 Additional comments (39)
LICENSE-THIRD-PARTY (1)
8-11: LGTM!.secrets.baseline (1)
136-136: LGTM!Also applies to: 354-358
docs/source/contributing/code-style.md (1)
24-25: 📐 Maintainability & Code QualityVerify the pre-commit matrix still matches the hook set.
pre-commit run --all-filesonly covers lock checks if explicit hooks are present; otherwise this doc now overstates the validation contributors get from the command.Source: Path instructions
scripts/dev.sh (1)
8-11: LGTM!Also applies to: 61-61
mcp/README.md (2)
109-115: 📐 Maintainability & Code QualityVerify the standalone wheel build command.
uv build mcp --wheelis a tool-specific invocation; please confirm it is valid for the pinneduvrelease, otherwise this doc will point contributors at a failing command.
3-108: LGTM!Also applies to: 117-154
.coderabbit.yaml (1)
99-103: LGTM!deploy/Dockerfile (1)
81-90: 🩺 Stability & AvailabilityVerify
uv pip installaccepts--no-sourceshere.If the pinned
uvrelease rejects that flag foruv pip install, the builder stage will fail at image build time.Source: Path instructions
docs/source/deployment/docker-build.md (1)
45-47: LGTM!Also applies to: 58-73
.agents/skills/aiq-maintain-ci/SKILL.md (1)
39-52: LGTM!CONTRIBUTING.md (1)
32-42: LGTM!.agents/skills/aiq-release-qa/references/validation-matrix.md (1)
16-19: LGTM!Also applies to: 50-62
scripts/setup.sh (1)
46-55: LGTM!Also applies to: 126-127
docs/source/contributing/pr-workflow.md (1)
30-36: LGTM!mcp/pyproject.toml (1)
55-81: LGTM!Verified the scoped-override table syntax (
{ package = { name, version }, dependencies = [...] }) against uv's documented resolution behavior — this is valid and matches the documented pattern for constraining a specific package's transitive dependency.README.md (1)
305-326: LGTM!The
cryptography<47(root) vscryptography==48.0.1(MCP frozen profile) distinction is consistent with the override comment inmcp/pyproject.toml.mcp/Dockerfile (1)
1-52: LGTM!The multi-stage layout (build wheels into
/opt/venvvia--no-editable --frozen, then copy only the venv/config/license into a minimal non-root release stage) is a sound pattern, and the port/health-check/entrypoint wiring lines up with theaiq-mcpcompose service referencing this Dockerfile.Also applies to: 56-93
.github/workflows/ci.yml (3)
72-72: Unpinnedpostgres:16-alpineservice image andactions/upload-artifact@v4references (new instances of a previously-flagged pattern).This PR adds a third
actions/upload-artifact@v4use (mcp-release-evidence, mcp-compose-logs) and keepspostgres:16-alpineunpinned, consistent with the earlier review comment on this file about pinning images/actions to immutable digests.Also applies to: 154-154, 284-284, 313-313
242-266: 🔒 Security & Privacy | ⚡ Quick winVerify
uv audit/uv exportpreview-feature flag names and the scope of the ignored advisory.Two things worth double-checking here:
- uv audit is a preview feature, and running it as-is will produce an experimental warning. Add --preview-features audit only if you want to suppress the warning. The workflow instead passes
--preview-features audit-command,json-output, which doesn't match the documentedauditfeature name. Unrecognized preview feature names only warn, they don't fail the build, but the intended warning-suppression likely isn't happening.--ignore-until-fixed GHSA-f4j7-r4q5-qw2cpermanently (until a fix appears) exempts this advisory from the production-lock gate; the public advisory record for this ID has no known source code · Dependabot alerts are not supported on this advisory because it does not have a package from a supported ecosystem with an affected and fixed version, so it's worth confirming this ID actually maps to a real MCP dependency vulnerability and not a stale/incorrect reference.
37-53: LGTM! Postgres service wiring, isolated MCP venv provisioning, mandatory 90%-coverage MCP test lane with skipped-test guard, SBOM/license evidence, and Compose smoke test/teardown are otherwise well-structured.Also applies to: 67-165, 184-322
docs/source/contributing/testing.md (1)
10-17: LGTM!.pre-commit-config.yaml (2)
84-89: 📐 Maintainability & Code Quality | ⚡ Quick winVerify
pytest-mcppush-stage hook is runnable locally without CI's dedicated Postgres.This hook runs
pytest mcp/testswith--cov-fail-under=90, but the CItestjob only provisions Postgres via a service container andAIQ_MCP_TEST_DB_URL. Ifmcp/testsrequires a live Postgres and a contributor runspre-commit run --all-files --hook-stage pushlocally without one configured, the hook will fail with no guidance in the docs.
61-66: 🚀 Performance & ScalabilityConfirm
uv-lock-mcpcovers every MCP path dependency. The currentfilesregex only watchessources/knowledge_layerandsources/tavily_web_search; ifmcp/pyproject.tomldepends on any othersources/*package, edits there will not retriggeruv lock --project mcp --check..agents/skills/aiq-release-qa/SKILL.md (1)
38-41: LGTM!Also applies to: 72-87, 104-108
.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md (1)
16-19: LGTM!Also applies to: 40-50
AGENTS.md (2)
43-43: LGTM!Also applies to: 55-61
105-109: 📐 Maintainability & Code QualityAGENTS.md:105-109 — align the
cryptographynote withpyproject.toml
This says the root workspace stays oncryptography>=46.0.6,<47while onlymcp/uses48.0.1, but the dependency change in this PR appears to target the root project. One of these is stale.mcp/src/aiq_mcp/jobs.py (1)
373-404: LGTM! Heartbeat resilience fix looks correct: transient store failures no longer kill the heartbeat loop, and_FAILURE_HANDLER/context cleanup is now guaranteed via the nestedfinallyeven if heartbeat teardown raises. This resolves the previously flagged critical issue.pyproject.toml (2)
100-100: LGTM!Also applies to: 157-157, 184-185, 242-242
211-226: 📐 Maintainability & Code QualityClarify the
cryptographyconstraint andmcp-testsgroup. If the change is meant to affect the rootmcp-testsgroup rather than[project].dependencies, add a short note explaining why this root test-only pin is needed whilemcp/remains excluded from the workspace.mcp/tests/test_dependency_compatibility.py (1)
26-31: LGTM!docs/source/integration/mcp-server.md (1)
32-52: LGTM!Dependency-isolation and container-deployment guidance is internally consistent with
mcp/SECURITY.mdand the packaging contract tests (cryptography floor/override split,mcp/uv.lockusage, loopback-only Compose stack).Also applies to: 221-249
mcp/tests/test_deployment_assets.py (1)
38-50: LGTM!Strict allowlisted
COPYline contract plus the new root-Dockerfile--no-sources --no-deps -e .assertion give solid drift detection for the multi-stage build.Also applies to: 62-96
mcp/SECURITY.md (1)
27-52: LGTM!The
uv export/uv auditpreview-feature flags (sbom-export,audit-command,json-output) match documented uv usage, and the cryptography override rationale is consistent with the packaging tests.Also applies to: 80-101
mcp/tests/test_config_and_packaging.py (1)
129-216: LGTM!Root/MCP lock and source-of-truth split (workspace exclusion, scoped cryptography override, editable path mapping) is thoroughly and correctly asserted.
mcp/tests/test_release_checks.py (1)
176-220: LGTM!mcp/tests/test_jobs.py (1)
676-760: LGTM!Good coverage: transient heartbeat failures retry without leaking the simulated credential or raw job id, and unexpected heartbeat-task crashes still force cleanup with sanitized logs. Directly exercises the no-secret-leakage requirement.
mcp/scripts/check_license_inventory.py (2)
214-236: LGTM!
validate_lock_sources()correctly rejects any non-registry, non-editable source and enforces the exact local-path/registry contract; wiring it beforebuild_inventory()inmain()is the right order.Also applies to: 433-448
189-211: 🗄️ Data Integrity & IntegrationCheck
uv:workspace:pathhandling for local MCP packages.validate_sbomrejects any purl-less component withuv:workspace:path; ifuv export --preview-features sbom-exportstarts tagging the approved path/editable deps frommcp/pyproject.toml, CI will fail them even though they are allowed.
#### Overview Refresh the AI-Q documentation against the live 2.2 milestone and current `develop` implementation while keeping branch-facing documentation portable across future release cuts. - keeps the root README version-free: “What’s New” highlights current capabilities without embedding release numbers, RC status, or a branch-cut lifecycle - keeps version-specific 2.2 targeting in the existing changelog, which is the detailed unreleased ledger; no separate release-notes document is introduced - makes the roadmap describe implementation in the checked-out branch rather than implying availability in a published release - updates configuration, deployment, quick-start, profiling, and docs-navigation wording to describe current behavior instead of a “2.2 candidate” - documents the newly merged artifact lifecycle (#314), Helm release-namespace behavior (#309), and async trace hierarchy (#321) - keeps the remaining open capabilities explicit: no per-job isolated/attested OpenShell lifecycle (#298) and no standalone public AI-Q MCP server (#319) - resolves Linette's review feedback across link-referral wording, terminology, and documentation clarity - preserves runtime boundaries for advisory routing, focused configuration profiles, MCP reconnect behavior, narrow forward-only encryption, best-effort artifact capture, and best-effort tokenomics phase attribution Milestone audit as of July 10, 2026: 49 items (47 PRs and 2 issues), including 41 PRs merged to `develop`, one PR merged only to `release/2.1`, three open PRs (#298, #319, and this documentation PR), and two closed-unmerged PRs superseded by merged work. AI-Q `v2.1.0` remains the latest stable release. `v2.2.0-rc1` is a prerelease snapshot from `develop`; there is no final `v2.2.0` tag yet, and `release/2.2` has not been cut. These lifecycle details intentionally remain outside the develop-facing README. #### Validation - `make -C docs SPHINXBUILD=../.venv/bin/sphinx-build SPHINXOPTS='-W --keep-going -n' html` - `make -C docs SPHINXBUILD=../.venv/bin/sphinx-build linkcheck` - `pre-commit run --all-files` - `git diff --check origin/develop...HEAD` - independent release-lifecycle, version-free wording, milestone, review-thread, and semantic whole-diff reviews found no remaining Critical or Important issues - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. Not applicable: this PR changes documentation only. - [x] I updated documentation for user-facing or contributor-facing changes. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. #### Where should reviewers start? 1. `README.md` for the version-free “What’s New” highlights and current-branch roadmap semantics. 2. `CHANGELOG.md` for the detailed unreleased 2.2 ledger and release lifecycle. 3. `docs/source/architecture/agents/deep-researcher.md` for the routed planner/researcher/writer contract. 4. `docs/source/architecture/data-flow.md` and `docs/source/integration/rest-api.md` for artifact checkpoint, SSE, replay, and authorization semantics. 5. `docs/source/deployment/kubernetes.md` and `docs/source/deployment/observability.md` for the newly merged namespace and trace-hierarchy behavior. #### Related Issues - Relates to the [AI-Q v2.2 milestone](https://github.com/NVIDIA-AI-Blueprints/aiq/milestone/1). - Documents merged changes from #309, #314, and #321. - Keeps open #298 and #319 explicitly out of the candidate scope. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated “Unreleased” release notes targeting AI-Q v2.2.0, including deep research workflow changes, async job reporting, and sandbox/artifact behavior. * Added/expanded REST API documentation for event-derived job state and durable artifact listing/streaming. * Documented OpenSearch support for knowledge retrieval and multiple paper-search providers, plus refined configuration, guardrails, MCP OAuth behavior, and observability trace hierarchy. * **Chores** * Refreshed secrets baseline metadata timestamps/line references only. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Ajay Thorve <athorve@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mcp/src/aiq_mcp/job_store.py (1)
250-274: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
record_poll's fallback path doesn't re-checkprincipal.When the
UPDATE ... RETURNINGmisses (terminal state or principal mismatch), the fallbackreturn await self.get(job_id)returns the row for any principal, unlikewait_for_completioninjobs.py, which explicitly checksjob.principal != principal. Not exploitable today since every caller shares the constantanonymousprincipal, but it's an inconsistent, latent authorization gap if multi-principal support is ever added.🛡️ Proposed fix
if row is not None: return _row_to_job(row) - return await self.get(job_id) + job = await self.get(job_id) + if job is not None and job.principal != principal: + return None + return job🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/src/aiq_mcp/job_store.py` around lines 250 - 274, Update record_poll so its fallback lookup enforces the supplied principal before returning a job. After get(job_id), return the job only when it is absent or its principal matches principal; otherwise return None, while preserving the existing successful UPDATE path.mcp/tests/test_postgres_job_store.py (1)
640-670: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate Postgres test-infrastructure helpers across two test files.
_ensure_database,_maintenance_url,_quote_database_name, the safe-identifier regex, and thepostgres_urlfixture skip/reset lifecycle are reimplemented nearly identically in both files (this diff even makes the same one-line message tweak in both). A future correctness or security fix to identifier quoting would need to be applied twice.
mcp/tests/test_postgres_job_store.py#L640-L670: extract_ensure_database,_maintenance_url,_quote_database_name, and the identifier regex into a sharedmcp/tests/conftest.py(or a_pg_test_utilsmodule) and import it here.mcp/tests/test_checkpoint_todos.py#L420-L477: replace the duplicated_ensure_database/_maintenance_url/_quote_database_name/regex with the same shared utility.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/tests/test_postgres_job_store.py` around lines 640 - 670, Extract the shared Postgres test helpers _ensure_database, _maintenance_url, _quote_database_name, and the safe-identifier regex into one utility module, then import and reuse them in mcp/tests/test_postgres_job_store.py lines 640-670 and mcp/tests/test_checkpoint_todos.py lines 420-477. Remove the duplicated definitions while preserving each file’s existing postgres_url fixture skip/reset lifecycle and shared error-message behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 112-118: Update the “Validate isolated dependency environments”
workflow step to pass --verify-imports when invoking
mcp/scripts/check_runtime_dependencies.py with the production test-venv Python,
ensuring the import canary runs against the --no-dev --no-editable dependency
closure while preserving the existing dependency check.
In `@docs/source/contributing/pr-workflow.md`:
- Around line 30-36: Update the MCP validation section in pr-workflow.md to
include both lock checks: the root-project lock and the MCP project lock,
alongside the existing sync and pytest commands. Preserve the current MCP test
commands and ensure contributors are instructed to verify both lockfiles are
current.
In `@mcp/scripts/check_license_inventory.py`:
- Around line 214-236: Add tests for validate_lock_sources() covering the
approved local-source set and rejection of at least one unapproved registry or
malformed editable source. Use temporary lock files or equivalent fixtures so
the tests exercise parsing and validation through validate_lock_sources(),
including the expected success and ValueError paths.
In `@mcp/src/aiq_mcp/server.py`:
- Line 143: Implement per-caller rate limiting for the unauthenticated research
submission endpoint, covering the request paths around the handlers at lines
183-188, 215, and 413-423, rather than only enforcing max_query_chars. Prefer
the existing deployment’s reverse-proxy/ingress mechanism or add Starlette
middleware, and ensure excess requests are rejected before starting LLM/tool
research jobs.
---
Outside diff comments:
In `@mcp/src/aiq_mcp/job_store.py`:
- Around line 250-274: Update record_poll so its fallback lookup enforces the
supplied principal before returning a job. After get(job_id), return the job
only when it is absent or its principal matches principal; otherwise return
None, while preserving the existing successful UPDATE path.
In `@mcp/tests/test_postgres_job_store.py`:
- Around line 640-670: Extract the shared Postgres test helpers
_ensure_database, _maintenance_url, _quote_database_name, and the
safe-identifier regex into one utility module, then import and reuse them in
mcp/tests/test_postgres_job_store.py lines 640-670 and
mcp/tests/test_checkpoint_todos.py lines 420-477. Remove the duplicated
definitions while preserving each file’s existing postgres_url fixture
skip/reset lifecycle and shared error-message behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 1e4a7271-3b16-4d49-83fb-a9eca46bebc4
⛔ Files ignored due to path filters (2)
mcp/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.coderabbit.yaml.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineAGENTS.mdCONTRIBUTING.mdLICENSE-THIRD-PARTYREADME.mddeploy/Dockerfiledocs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/get-started/installation.mddocs/source/integration/mcp-server.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/Dockerfilemcp/README.mdmcp/REFERENCE_PARITY.mdmcp/SECURITY.mdmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/scripts/check_runtime_dependencies.pymcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_imports.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_release_checks.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_server_runtime.pypyproject.tomlscripts/dev.shscripts/setup.shskills/aiq-research/skill-card.mdtests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{py,ts,tsx,js,jsx,yml,yaml,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and
SecretStr, and resolve API keys at runtime.
Files:
docs/source/contributing/pr-workflow.mddocs/source/get-started/installation.mdCONTRIBUTING.mdmcp/REFERENCE_PARITY.mdmcp/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pydocs/source/contributing/code-style.mdskills/aiq-research/skill-card.mdREADME.mdAGENTS.mdtests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pydocs/source/contributing/testing.mdmcp/SECURITY.mddocs/source/integration/mcp-server.mddocs/source/deployment/docker-build.mdmcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update documentation when behavior, configuration, or workflows change; keep documentation canonical rather than duplicating full documentation pages into skills.
Files:
docs/source/contributing/pr-workflow.mddocs/source/get-started/installation.mddocs/source/contributing/code-style.mddocs/source/contributing/testing.mddocs/source/integration/mcp-server.mddocs/source/deployment/docker-build.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Stay within this repository, do not edit adjacent repositories, and keep changes tosources/*scoped to the smallest relevant independent package.
Run the narrowest relevant validation command first and broaden to the full suite only when a change crosses shared boundaries.
Keep the rootuv.lockandmcp/uv.lockas separate dependency resolutions; scope thecryptography==48.0.1override tomcp/, while the root usescryptography>=46.0.6,<47.
Keep pull requests scoped, avoid unrelated files and generated artifacts, do not include secrets, and provide validation evidence.
Every commit must include DCO sign-off usinggit commit -s, with aSigned-off-bytrailer.
For substantial behavior, authentication, UI, or architecture changes, open a design discussion before coding.
**/*: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts in contributions.
Add or update tests for behavior changes.
Run the narrowest local validation command that covers the change and include exact output or a workflow link in the pull request.
For deployment changes, run the relevant Helm or Compose validation and describe the environment used.
Target thedevelopbranch unless a maintainer explicitly requests a release branch.
Sign off every commit withgit commit -s; commits without sign-off may be rejected.
Open an issue or discussion before large design changes, public APIs, deployment changes, or contributor workflow changes.
Make the smallest coherent change and address review feedback until required checks and code-owner review pass.
Files:
docs/source/contributing/pr-workflow.mdLICENSE-THIRD-PARTYdocs/source/get-started/installation.mdCONTRIBUTING.mdscripts/setup.shmcp/REFERENCE_PARITY.mdmcp/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pydocs/source/contributing/code-style.mdskills/aiq-research/skill-card.mdREADME.mddeploy/Dockerfilemcp/DockerfileAGENTS.mdtests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pyscripts/dev.shmcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pydocs/source/contributing/testing.mdmcp/SECURITY.mddocs/source/integration/mcp-server.mdmcp/pyproject.tomldocs/source/deployment/docker-build.mdmcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pypyproject.tomlmcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
docs/source/contributing/**/*.{py,md,ipynb}
📄 CodeRabbit inference engine (docs/source/contributing/code-style.md)
Run pre-commit checks across all files; these include Ruff checks and formatting, lock-file checks, secret detection, notebook output clearing, and Markdown link validation.
Files:
docs/source/contributing/pr-workflow.mddocs/source/contributing/code-style.mddocs/source/contributing/testing.md
docs/source/contributing/**/*
📄 CodeRabbit inference engine (docs/source/contributing/pr-workflow.md)
For each change, run the narrowest relevant local validation checks and record the exact commands in the pull request description, including repository checks (
uv run ruff check .,uv run ruff format --check ., anduv run pytest), MCP checks when applicable, and UI checks when applicable.
Files:
docs/source/contributing/pr-workflow.mddocs/source/contributing/code-style.mddocs/source/contributing/testing.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/contributing/pr-workflow.mddocs/source/get-started/installation.mdCONTRIBUTING.mddocs/source/contributing/code-style.mdREADME.mddocs/source/contributing/testing.mddocs/source/integration/mcp-server.mddocs/source/deployment/docker-build.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Ruff for Python linting and formatting with line length 120, target Python 3.11, rule sets E, F, W, I, PL, and UP; use single-line imports as configured.
Never print or log secret values, including in tool output or error messages.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,pyi}: Runuv run ruff check .anduv run ruff format --check .for Python changes.
Runuv run pytestfor relevant Python changes.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.py
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/skill-card.md.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-release-qa/SKILL.md
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/Dockerfile
.agents/skills/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use maintainer skills from
.agents/skills/for task-specific repository workflows; do not treat API-consumer skills inskills/as an in-product runtime.
Files:
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-release-qa/SKILL.md
mcp/**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For changes under
mcp/, run the MCP project's development dependency sync anduv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
.pre-commit-config.yamlmcp/pyproject.toml.github/workflows/ci.ymlpyproject.toml
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: Create a focused branch from `develop` and open pull requests into `develop`, unless a maintainer directs the contributor to target a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: Address review feedback until required checks, code-owner review, and review-thread resolution pass before merge.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: All contributors must sign off their commits using the Developer's Certificate of Origin, for example with `git commit -s -m "Your message"`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: Contributions must satisfy the Developer's Certificate of Origin 1.1: the contributor must have the right to submit the work under the applicable open-source license, and acknowledges that contribution and sign-off records are public and retained indefinitely.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:53.553Z
Learning: Use the documented `uv` commands to install dependencies and run the main project or MCP project tests.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:53.553Z
Learning: Refer to each benchmark's README for benchmark-specific instructions and consult the customization guide when adding evaluation harnesses.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:53.553Z
Learning: Use verbose CLI logging, Phoenix tracing, and the documented troubleshooting steps when debugging.
🪛 ast-grep (0.44.1)
mcp/scripts/check_runtime_dependencies.py
[info] 134-134: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/tests/test_server_runtime.py
[warning] 1034-1034: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1083-1083: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1158-1158: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1202-1202: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
🪛 OpenGrep (1.25.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 166-181: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 SkillSpector (2.3.11)
.agents/skills/aiq-release-qa/SKILL.md
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🔇 Additional comments (59)
LICENSE-THIRD-PARTY (1)
8-11: LGTM!docs/source/get-started/installation.md (1)
15-15: LGTM!Also applies to: 49-49
CONTRIBUTING.md (1)
32-42: LGTM!scripts/setup.sh (3)
6-38: LGTM!
65-74: LGTM!Also applies to: 89-98
169-170: LGTM!mcp/REFERENCE_PARITY.md (2)
1-75: LGTM!
76-84: 📐 Maintainability & Code Quality
mcp/tests/test_workflow_runner.pyexists, so this reference is valid.> Likely an incorrect or invalid review comment.frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
872-888: LGTM!mcp/src/aiq_mcp/job_store.py (5)
1-106: LGTM!
107-183: LGTM!
184-234: LGTM!
275-358: LGTM!
359-393: LGTM!mcp/README.md (2)
1-179: LGTM! The rest of the documentation (component layout, anonymous capability model, Host/Origin/CORS guidance, dependency isolation, container deployment) is internally consistent and matches the coding guidelines for scopingcryptography==48.0.1tomcp/.
34-36: 🎯 Functional CorrectnessNo issue:
GET /mcpis explicitly rejected The app routesself.settings.pathto a handler that returns405withAllow: POST, so the README matches the implementation.> Likely an incorrect or invalid review comment..coderabbit.yaml (1)
99-99: LGTM! Expanding the automation/packaging path glob to covermcp/pyproject.tomlandmcp/uv.lockmatches the path instructions for reviewing packaging changes.docs/source/contributing/code-style.md (1)
24-25: LGTM!.secrets.baseline (1)
130-138: LGTM!Also applies to: 348-358
mcp/tests/test_dependency_compatibility.py (1)
26-31: LGTM! Thecryptographypin matches the documentedmcp/uv.lockoverride.mcp/tests/test_checkpoint_todos.py (1)
41-51: LGTM! Including onlytype(exc).__name__(not the raw exception message, which could contain the DSN) keeps the skip/warning message free of connection secrets.mcp/tests/test_postgres_job_store.py (2)
57-67: LGTM!
393-468: LGTM! This test precisely covers themark_failed_if_queued_or_ownedownership/state contract (queued, self-owned running, foreign-owned running, complete, already-failed), matching the guarded UPDATE predicate injob_store.py.skills/aiq-research/skill-card.md (1)
33-35: LGTM!README.md (2)
234-244: LGTM!Also applies to: 305-326
312-315: 🎯 Functional CorrectnessThe
aiq-mcp-serverconsole script is declared inmcp/pyproject.toml, so the README command is valid.> Likely an incorrect or invalid review comment.deploy/Dockerfile (1)
81-90: LGTM!--no-sourcescorrectly prevents uv from attempting workspace-member discovery for[tool.uv.sources]entries during the--no-deps -e .install, matching the stated rationale.mcp/Dockerfile (1)
10-57: LGTM! Minimal, well-justified copy set and frozen--no-dev --no-default-groups --no-editablesync for the release closure; the import canary correctly wiresAIQ_MCP_CONFIGto the copied config..agents/skills/aiq-maintain-ci/SKILL.md (1)
39-52: LGTM!mcp/tests/test_release_checks.py (1)
176-238: LGTM! These three tests matchvalidate_sbom's actual branch logic (local-source allowlist check vs. public PyPI purl contract vs. final set-equality check).mcp/scripts/check_license_inventory.py (1)
178-213: LGTM!validate_sbomlogic matches the exact test expectations reviewed inmcp/tests/test_release_checks.py.mcp/tests/test_jobs.py (1)
67-182: LGTM! Solid coverage of claim/cancellation races, heartbeat retry, and log/error redaction — verified against the realJobManager/JobStorecontracts referenced in this review's graph context.Also applies to: 665-1172
AGENTS.md (1)
43-43: LGTM!Also applies to: 55-61
tests/aiq_agent/jobs/test_runner.py (1)
886-894: LGTM!mcp/tests/test_deployment_assets.py (1)
16-16: LGTM!Also applies to: 38-50, 62-88, 91-96
.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md (1)
13-19: LGTM!Also applies to: 38-51
.agents/skills/aiq-release-qa/references/validation-matrix.md (1)
13-19: LGTM!Also applies to: 50-62
pyproject.toml (2)
184-185: LGTM!
50-50: 🗄️ Data Integrity & Integration | ⚡ Quick winRoot
cryptographyconstraint: verify[project].dependenciesvs.[tool.uv.override-dependencies]and documentation. The change summary sayspyproject.toml's directcryptographydependency moved to>=48.0.1,<49, but the shownoverride-dependenciesstill pins>=46.0.6,<47, andAGENTS.mddocuments the root staying on<47. Since uv'soverride-dependenciesforces the resolved version regardless of the direct dependency declaration, either the bump is a no-op that should be removed/reverted, or the override needs to be updated and the docs are now stale.
pyproject.toml#L50-L50: confirm whether the[project].dependenciescryptography bump to>=48.0.1,<49is intentional; if so, update or remove the conflicting>=46.0.6,<47entry in[tool.uv.override-dependencies]so the direct dependency isn't silently overridden.AGENTS.md#L105-L109: update the documented root cryptography range if the intended behavior changes, or leave as-is oncepyproject.tomlis confirmed to still resolve to<47in practice.Source: Coding guidelines
mcp/tests/test_server_runtime.py (1)
163-249: LGTM!Also applies to: 299-300, 925-943, 977-1222
mcp/src/aiq_mcp/jobs.py (1)
211-232: LGTM! Confirms the previously flagged heartbeat-retry issue is fixed, and the cancellation/failure-persistence paths correctly rely on guarded state-conditioned store writes to avoid clobbering already-terminal jobs.Also applies to: 317-325, 368-424
.agents/skills/aiq-release-qa/SKILL.md (2)
111-112: Static-analysis false positive.The "credential access" flag on the
deploy/.envreference is a false positive — this is a doc instructing contributors how to invokenat evalwithdotenv, not code that reads/logs a secret file.
38-41: LGTM!Also applies to: 72-88, 104-108
mcp/scripts/check_runtime_dependencies.py (2)
134-134: Static-analysis false positive.The "use jsonify instead of json.dumps" hint doesn't apply — this is a standalone CLI script printing to stdout, not a web response body.
108-138: LGTM!mcp/tests/test_runtime_dependencies.py (1)
80-119: LGTM!scripts/dev.sh (1)
6-13: LGTM!Also applies to: 61-61
mcp/tests/test_imports.py (1)
32-41: LGTM!.github/workflows/ci.yml (2)
249-256: 🔒 Security & PrivacyConfirm the ignored advisory is still unresolved and scoped correctly.
GHSA-f4j7-r4q5-qw2cis suppressed via--ignore-until-fixed, which auto-reinstates the gate once a fix ships. Worth a periodic sanity check (or a tracked follow-up) that this advisory still applies to a real dependency inmcp/uv.lockand hasn't been superseded/fixed, since GitHub's advisory record for this ID shows no associated ecosystem package.
37-53: LGTM!Also applies to: 67-85, 99-119, 120-139, 140-165, 187-237, 257-321
mcp/tests/test_client_session_integration.py (2)
443-446: 🔒 Security & PrivacyPreviously flagged raw-exception-in-message issue is fixed here (
type(exc).__name__instead ofexc).
133-140: LGTM!docs/source/contributing/testing.md (1)
10-17: LGTM!.pre-commit-config.yaml (1)
25-31: LGTM!Also applies to: 61-66, 78-89
mcp/SECURITY.md (1)
6-133: LGTM!docs/source/integration/mcp-server.md (1)
18-274: LGTM!docs/source/deployment/docker-build.md (1)
39-73: LGTM!mcp/tests/test_config_and_packaging.py (1)
158-224: LGTM!mcp/pyproject.toml (1)
59-77: 🔒 Security & PrivacyKeep the current prerelease policy.
mcp/SECURITY.mdalready documents prerelease-aware resolution as part of the MCP release profile, and thedefusedxmlcap exists specifically because of that policy.tornado>=6.5.5can stay open if that release line is meant to float.> Likely an incorrect or invalid review comment.
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
Review against the current head, including an isolated release-container run with the credentials from deploy/.env.
The unit/protocol suites and image build are green, but the credentialed shallow and deep paths both returned generic error text while the MCP ledger reported state="complete". The inline comments cover that state-contract failure, the standalone shallow runtime failure, retention leakage, duplicate classification, the non-installable wheel, the dropped Compose setting, and the missing report-follow-up surface.
GitHub also currently reports this head as CONFLICTING / DIRTY against develop (.secrets.baseline, README.md, and uv.lock), so the checks need to be rerun after the branch is updated.
Add an anonymous MCP transport for submitting and polling AI-Q research jobs, backed by PostgreSQL. Include deployment assets, documentation, compatibility tests, release checks, and CI gates. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Require NLTK 3.10, constrain its new defusedxml dependency to the stable 0.7 release line, and refresh the lockfile. Remove the resolved NLTK vulnerability exceptions from the MCP audit gate and update the release security documentation. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Sanitize captured workflow failures before returning them through public MCP poll/final-report responses, and guard async job state transitions by current state and runner ownership to avoid stale background tasks overwriting terminal rows. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Move the MCP server into an opt-in dependency group and select it in CI, setup, pre-commit, and full-suite validation paths. Keep the main AI-Q Docker build independent of absent MCP workspace sources, and add regression coverage and updated usage documentation. Signed-off-by: Tanner Leach <tleach@nvidia.com>
- split MCP from the root uv workspace and add its own lock - keep cryptography 48 limited to the audited MCP release profile - route CI, Docker, SBOM, tests, and docs through the MCP project Signed-off-by: Tanner Leach <tleach@nvidia.com>
Retry heartbeat writes after transient store failures without exposing internal exception details. Ensure heartbeat task failures cannot skip job context and failure-capture cleanup. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Require uv 0.11.25 or newer for scoped dependency overrides and make setup reject unsupported versions before environment creation. Align the documentation and packaging contracts, and correct the import boundary assertion for the isolated MCP environment. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Sanitize submission, polling, and report infrastructure exceptions before FastMCP serializes them. Preserve the original queued capability when an inline wait fails, add transport-level sentinel coverage, and document the stable failure contract. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Validate queued submit response fields before inline waits so malformed job data returns a public MCP error without exposing internal details. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Pin the MCP PostgreSQL service image and release artifact uploads to immutable references. Avoid exposing raw PostgreSQL connection errors from integration-test fixtures. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Guard failure persistence so queued jobs and running jobs owned by the current runner can transition to failed without overwriting terminal or foreign-owned work. Keep shallow submissions pollable if a local task exits with a nonterminal row, and cover claim and cancellation races in unit and PostgreSQL tests. Signed-off-by: Tanner Leach <tleach@nvidia.com>
The job runner now persists only the exception class name instead of raw exception text, so the encrypted event-flush failure test must assert the sanitized error message rather than the injected RuntimeError text. Signed-off-by: Tanner Leach <tleach@nvidia.com>
tanleach
force-pushed
the
feat/mcp-phase1
branch
from
05882bf to
df763f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/aiq_agent/knowledge/summary_store.py (1)
141-154: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore the redacted engine identity in the dispose-failure warning.
Line 153 now logs only
type(e).__name__, dropping the_redact_db_url(key)reference that the success-path log (line 151) still includes right above it. Since the key was already going through_redact_db_url, re-adding it costs nothing and restores the ability to tell which cached engine failed to dispose.🔧 Proposed fix
except Exception as e: - logger.warning("Failed to dispose engine (%s)", type(e).__name__) + logger.warning("Failed to dispose engine for %s (%s)", _redact_db_url(key), type(e).__name__)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiq_agent/knowledge/summary_store.py` around lines 141 - 154, Update the exception warning in _cleanup_stale_engines to include the redacted engine identity via _redact_db_url(key), while retaining the exception type and existing success-path behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcp/README.md`:
- Line 81: Document the AIQ_MCP_MAX_QUERY_CHARS setting in both configuration
tables in mcp/README.md, including its tested default value of 8000 and a
concise description consistent with the surrounding entries. Do not alter the
existing settings.
- Around line 79-80: Update the environment-variable defaults table in the MCP
README to explicitly mark the configs/config_mcp.yml and deploy/.env defaults
for AIQ_MCP_CONFIG and AIQ_MCP_ENV_FILE as source-checkout-only. Clarify that
installed-package or wheel usage does not receive these paths by default and
must provide AIQ_MCP_CONFIG as required.
In `@mcp/REFERENCE_PARITY.md`:
- Line 26: Update the tool-surface freeze documentation and parity definition to
account for report follow-up capabilities such as report_ask and report_edit,
either by including them in the frozen surface or by documenting the intentional
product gap and planned release. Ensure the SHA-256 parity hash and the stated
tool ordering/schema remain consistent with the chosen surface.
In `@mcp/scripts/check_license_inventory.py`:
- Line 1: Add test coverage for validate_lock_sources by exposing it through the
_NAMESPACE setup in test_release_checks.py and exercising a temporary lock file
with the exact approved local-source set, an unapproved registry source, and a
non-string or malformed editable value. Ensure the accepted fixture explicitly
verifies whether the aiq-agent "../" versus ".." path normalization is handled
correctly, without changing validate_lock_sources unless the test demonstrates
an actual path-matching defect.
In `@mcp/scripts/protocol_smoke.py`:
- Around line 29-33: Update _health_url to reject endpoint URLs containing
userinfo or query parameters (and fragments) before constructing the health URL,
preserving only valid absolute HTTP/HTTPS URLs without credentials or tokens.
Ensure the smoke-test output path around the endpoint handling also does not
echo the original credential-bearing URL or expose secret values.
In `@mcp/src/aiq_mcp/job_store.py`:
- Around line 235-273: Update JobStore.get to require a principal argument and
constrain its query with both job_id and principal. In record_poll, pass the
provided principal to the fallback get call so failed principal-scoped updates
cannot return another principal’s job data.
In `@mcp/src/aiq_mcp/jobs.py`:
- Around line 60-135: Replace MCP-layer inference in _WorkflowFailureCapture and
_run_job with a structured terminal outcome emitted by the workflow boundary,
containing success or failed status and a stable public error for every
swallowed research failure. Persist that outcome directly so fallback responses
cannot be saved as state="complete"; remove reliance on _CAPTURED_EXCEPTIONS and
log-message substring matching.
- Line 241: Remove the duplicate classification between submit() and
WorkflowRunner.run_query(): make one component own the classifier result and
thread that decision through _run_job into the workflow instead of invoking
intent_classifier again. Ensure the persisted depth and duration correspond to
the route actually executed, and add a test covering that consistency for a
non-deterministic classifier.
In `@mcp/src/aiq_mcp/workflow_runner.py`:
- Around line 62-88: The _close_owned_checkpointers method relies on private
aiq_agent.common cache names and silently skips cleanup when they are absent.
Replace this dependency with a public checkpointer cleanup or iteration API; if
no such API exists, detect missing _checkpointers or _postgres_pools and emit a
warning before handling cleanup, while preserving ownership-based removal and
closing behavior.
In `@mcp/tests/test_postgres_job_store.py`:
- Around line 651-655: Update the _reset_schema helper to validate db_url before
opening the connection or executing DROP statements, and fail closed unless the
target database is the CI/test database named aiq_mcp_tests. Keep the
destructive reset behavior unchanged for that explicitly allowed database and
reject all other targets.
---
Outside diff comments:
In `@src/aiq_agent/knowledge/summary_store.py`:
- Around line 141-154: Update the exception warning in _cleanup_stale_engines to
include the redacted engine identity via _redact_db_url(key), while retaining
the exception type and existing success-path behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2b17bbf0-40af-4bb1-bc97-c57b368ebf32
⛔ Files ignored due to path filters (2)
mcp/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.coderabbit.yaml.dockerignore.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineAGENTS.mdCONTRIBUTING.mdLICENSE-THIRD-PARTYREADME.mdconfigs/config_mcp.ymldeploy/Dockerfiledeploy/compose/docker-compose.mcp.yamldocs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/get-started/installation.mddocs/source/index.mddocs/source/integration/index.mddocs/source/integration/mcp-server.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/Dockerfilemcp/LICENSEmcp/README.mdmcp/REFERENCE_PARITY.mdmcp/SECURITY.mdmcp/deploy/README.mdmcp/deploy/init-mcp-db.sqlmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/scripts/check_runtime_dependencies.pymcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/__init__.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/src/aiq_mcp/db_url.pymcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/conftest.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_db_url.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_imports.pymcp/tests/test_job_store.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_release_checks.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_server_runtime.pymcp/tests/test_workflow_runner.pypyproject.tomlscripts/dev.shscripts/setup.shskills/aiq-research/skill-card.mdsrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/common/logging_utils.pysrc/aiq_agent/knowledge/summary_store.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/jobs/test_runner.pytests/knowledge_layer_tests/test_summary_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Pytest and Coverage
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{py,ts,tsx,js,jsx,yml,yaml,json,env,md}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and
SecretStr, and resolve API keys at runtime.
Files:
docs/source/integration/index.mdmcp/src/aiq_mcp/__init__.pyskills/aiq-research/skill-card.mddocs/source/index.mdmcp/tests/conftest.pymcp/tests/test_db_url.pymcp/deploy/README.mdmcp/tests/test_job_store.pydocs/source/get-started/installation.mdmcp/tests/test_dependency_compatibility.pyAGENTS.mdtests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/REFERENCE_PARITY.mdmcp/src/aiq_mcp/db_url.pydocs/source/contributing/code-style.mdCONTRIBUTING.mdtests/aiq_agent/common/test_logging_utils.pyconfigs/config_mcp.ymltests/aiq_agent/jobs/test_runner.pydocs/source/contributing/pr-workflow.mdmcp/README.mddocs/source/deployment/docker-build.mdmcp/tests/test_imports.pydeploy/compose/docker-compose.mcp.yamlfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/knowledge_layer_tests/test_summary_store.pyREADME.mdmcp/scripts/check_runtime_dependencies.pysrc/aiq_agent/common/logging_utils.pydocs/source/contributing/testing.mdmcp/src/aiq_mcp/workflow_runner.pymcp/SECURITY.mdmcp/tests/test_checkpoint_todos.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_deployment_assets.pymcp/tests/test_workflow_runner.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/scripts/protocol_smoke.pymcp/tests/test_server_runtime.pymcp/tests/test_runtime_dependencies.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/checkpoint_todos.pydocs/source/integration/mcp-server.mdmcp/src/aiq_mcp/job_store.pymcp/scripts/check_license_inventory.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update canonical documentation under
docs/source/when behavior, configuration, or workflows change; do not duplicate full documentation pages into skills.
Files:
docs/source/integration/index.mddocs/source/index.mddocs/source/get-started/installation.mddocs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/deployment/docker-build.mddocs/source/contributing/testing.mddocs/source/integration/mcp-server.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes scoped to this repository and avoid editing adjacent repositories; treat eachsources/*package as independent and prefer the smallest package-local change.
Run the narrowest relevant validation command first, broadening to the full suite only when a change crosses shared boundaries.
For substantial behavior, authentication, UI, or architecture changes, open a design discussion before coding.
Keep pull requests scoped: avoid unrelated files, accidental generated artifacts, and secrets, and provide validation evidence with commands and results.
Every commit must be signed off withgit commit -s; commits must contain aSigned-off-bytrailer.
Load the relevant task-specific maintainer skill from.agents/skills/before starting a workflow it covers; API-consumer skills underskills/are a separate audience.
Files:
docs/source/integration/index.mdmcp/LICENSEmcp/src/aiq_mcp/__init__.pyskills/aiq-research/skill-card.mdLICENSE-THIRD-PARTYdocs/source/index.mdmcp/tests/conftest.pymcp/tests/test_db_url.pymcp/deploy/README.mddeploy/Dockerfilemcp/tests/test_job_store.pydocs/source/get-started/installation.mdmcp/tests/test_dependency_compatibility.pyAGENTS.mdtests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/REFERENCE_PARITY.mdmcp/src/aiq_mcp/db_url.pydocs/source/contributing/code-style.mdCONTRIBUTING.mdscripts/dev.shtests/aiq_agent/common/test_logging_utils.pyconfigs/config_mcp.ymltests/aiq_agent/jobs/test_runner.pymcp/deploy/init-mcp-db.sqldocs/source/contributing/pr-workflow.mdmcp/README.mddocs/source/deployment/docker-build.mdmcp/tests/test_imports.pydeploy/compose/docker-compose.mcp.yamlfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/knowledge_layer_tests/test_summary_store.pyREADME.mdmcp/Dockerfilemcp/scripts/check_runtime_dependencies.pysrc/aiq_agent/common/logging_utils.pyscripts/setup.shdocs/source/contributing/testing.mdmcp/src/aiq_mcp/workflow_runner.pymcp/SECURITY.mdmcp/tests/test_checkpoint_todos.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.pymcp/pyproject.tomlmcp/tests/test_deployment_assets.pymcp/tests/test_workflow_runner.pypyproject.tomlmcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/scripts/protocol_smoke.pymcp/tests/test_server_runtime.pymcp/tests/test_runtime_dependencies.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/checkpoint_todos.pydocs/source/integration/mcp-server.mdmcp/src/aiq_mcp/job_store.pymcp/scripts/check_license_inventory.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.py
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/integration/index.mddocs/source/index.mddocs/source/get-started/installation.mddocs/source/contributing/code-style.mdCONTRIBUTING.mddocs/source/contributing/pr-workflow.mddocs/source/deployment/docker-build.mdREADME.mddocs/source/contributing/testing.mddocs/source/integration/mcp-server.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Format and lint Python with Ruff using the repository’s configuration: line length 120, target Python 3.11, rules E/F/W/I/PL/UP, and single-line imports.
Never print or log secret values, including in tool output or error messages.
Missing-secret paths must degrade gracefully by stubbing or skipping rather than crashing or leaking values.For Python changes, run
uv run ruff check .,uv run ruff format --check ., anduv run pytestas applicable.
Files:
mcp/src/aiq_mcp/__init__.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/tests/test_job_store.pymcp/tests/test_dependency_compatibility.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/src/aiq_mcp/db_url.pytests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/jobs/test_runner.pymcp/tests/test_imports.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/knowledge_layer_tests/test_summary_store.pymcp/scripts/check_runtime_dependencies.pysrc/aiq_agent/common/logging_utils.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_checkpoint_todos.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_deployment_assets.pymcp/tests/test_workflow_runner.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/scripts/protocol_smoke.pymcp/tests/test_server_runtime.pymcp/tests/test_runtime_dependencies.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/src/aiq_mcp/job_store.pymcp/scripts/check_license_inventory.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Validate UI-affecting changes with
npm run lint,npm run type-check, andnpm run test:ci; include a screenshot for visible changes.
Files:
mcp/src/aiq_mcp/__init__.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/tests/test_job_store.pymcp/tests/test_dependency_compatibility.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/src/aiq_mcp/db_url.pytests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/jobs/test_runner.pymcp/tests/test_imports.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/knowledge_layer_tests/test_summary_store.pymcp/scripts/check_runtime_dependencies.pysrc/aiq_agent/common/logging_utils.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_checkpoint_todos.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_deployment_assets.pymcp/tests/test_workflow_runner.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/scripts/protocol_smoke.pymcp/tests/test_server_runtime.pymcp/tests/test_runtime_dependencies.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/src/aiq_mcp/job_store.pymcp/scripts/check_license_inventory.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.py
mcp/**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For changes under
mcp/, run the MCP project's development setup anduv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/src/aiq_mcp/__init__.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/tests/test_job_store.pymcp/tests/test_dependency_compatibility.pymcp/src/aiq_mcp/db_url.pymcp/tests/test_imports.pymcp/scripts/check_runtime_dependencies.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_deployment_assets.pymcp/tests/test_workflow_runner.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/scripts/protocol_smoke.pymcp/tests/test_server_runtime.pymcp/tests/test_runtime_dependencies.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/src/aiq_mcp/job_store.pymcp/scripts/check_license_inventory.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.py
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/skill-card.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-release-qa/SKILL.md
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/Dockerfileconfigs/config_mcp.ymldeploy/compose/docker-compose.mcp.yaml
configs/**/*.y{a,}ml
📄 CodeRabbit inference engine (AGENTS.md)
Use
_typenames derived from the registered configuration class for NAT YAML configuration schemas.
Files:
configs/config_mcp.yml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For deployment changes, run the relevant Helm or Compose validation and describe the environment used.
Files:
configs/config_mcp.ymldeploy/compose/docker-compose.mcp.yaml
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Respect authenticated data sources: honorrequires_auth, pass through per-user tokens, use backend token validators, and apply owner guardrails before loading protected report or artifact context into an agent.
Do not weaken or bypassAuthMiddleware, token validators, or authentication gating without prior design discussion.
Files:
src/aiq_agent/common/logging_utils.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
.pre-commit-config.yamlmcp/pyproject.tomlpyproject.toml.github/workflows/ci.yml
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/register.py
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
src/aiq_agent/knowledge/summary_store.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Search existing issues and pull requests before opening new work.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Open an issue or discussion before making large design changes, public API changes, deployment changes, or contributor workflow changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts in contributions.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Target the `develop` branch unless a maintainer directs use of a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Keep changes focused and add or update tests for behavior changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Sign off every commit with `git commit -s`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Run relevant local validation before opening a pull request and include exact validation output or a workflow link.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Pull requests must target `develop`, use the PR template with exact validation evidence, and satisfy required checks, code-owner review, resolved review threads, and branch policy before merge.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:53.280Z
Learning: Contributors must ensure their contribution is original or appropriately licensed and that they have the right to submit it under the project's license.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:40:56.506Z
Learning: Run `pre-commit run --all-files` or the repository's `./scripts/dev.sh pre-commit` workflow before submitting changes; this includes linting, formatting, lock checks, secret detection, notebook output clearing, and Markdown link validation.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:41:08.847Z
Learning: Use the documented `uv` commands to install development dependencies, run pytest, generate coverage, or execute an individual test file.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:41:08.847Z
Learning: Refer to each benchmark's README for benchmark-specific testing details.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:41:08.847Z
Learning: Use the Customization guide when adding evaluation harnesses.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T14:41:08.847Z
Learning: For debugging, use verbose CLI logging, Phoenix tracing, and the documented remedies for common import, authentication, tool, and pre-commit issues.
🪛 ast-grep (0.44.1)
mcp/scripts/check_runtime_dependencies.py
[info] 134-134: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/scripts/protocol_smoke.py
[info] 137-137: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/tests/test_server_runtime.py
[warning] 521-521: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 588-588: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 601-601: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 608-608: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 670-670: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 682-682: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 687-687: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 703-703: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 737-737: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 752-752: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 763-763: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://untrusted.example"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1034-1034: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1083-1083: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1158-1158: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1202-1202: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1241-1241: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 763-763: Do not make http calls without encryption
Context: "http://untrusted.example"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[info] 442-442: use jsonify instead of json.dumps for JSON output
Context: json.dumps(schema_contract, sort_keys=True, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/src/aiq_mcp/server.py
[warning] 78-78: Do not make http calls without encryption
Context: "http://[::1]:*"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 79-79: Do not make http calls without encryption
Context: "http://0.0.0.0:*"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
mcp/scripts/check_license_inventory.py
[info] 173-173: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload, sort_keys=True, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 445-445: use jsonify instead of json.dumps for JSON output
Context: json.dumps(inventory, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 446-446: use jsonify instead of json.dumps for JSON output
Context: json.dumps(validation, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Checkov (3.3.8)
mcp/Dockerfile
[low] 10-10: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
🪛 Hadolint (2.14.0)
mcp/Dockerfile
[warning] 16-16: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[warning] 64-64: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
🪛 OpenGrep (1.25.0)
mcp/tests/test_checkpoint_todos.py
[ERROR] 426-426: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_client_session_integration.py
[ERROR] 510-510: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_postgres_job_store.py
[ERROR] 646-646: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/src/aiq_mcp/job_store.py
[ERROR] 121-137: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 142-154: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 166-181: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 186-197: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 229-232: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 277-277: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 282-293: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 297-297: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 298-307: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 308-326: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 327-329: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 330-332: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 333-333: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 334-336: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 337-339: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 340-348: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 349-357: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 SkillSpector (2.3.11)
.agents/skills/aiq-release-qa/SKILL.md
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🪛 SQLFluff (4.2.2)
mcp/deploy/init-mcp-db.sql
[error] 43-43: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 44-44: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 45-45: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 46-46: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/aiq_agent/agents/chat_researcher/agent.py (1)
551-568: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReset turn-scoped state for dict inputs.
run()accepts raw dict state, but this branch forwardsworkflow_outcomeandshallow_resultunchanged. With a checkpointedthread_id, LangGraph merges partial dict input into the prior snapshot, so omitted keys can carry a previous turn’s failure state into the next response. Mirror theChatResearcherStatebranch here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiq_agent/agents/chat_researcher/agent.py` around lines 551 - 568, Update the dict-input branch in run() to explicitly reset turn-scoped shallow_result and workflow_outcome to None, matching the ChatResearcherState branch before LangGraph merges checkpointed state. Preserve the existing messages extraction and other dict-provided values.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 551-568: Update the dict-input branch in run() to explicitly reset
turn-scoped shallow_result and workflow_outcome to None, matching the
ChatResearcherState branch before LangGraph merges checkpointed state. Preserve
the existing messages extraction and other dict-provided values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 4114dfdd-c6be-48b2-92b1-ab278af1e6ab
📒 Files selected for processing (14)
mcp/REFERENCE_PARITY.mdmcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_client_session_integration.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_workflow_runner.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/models/__init__.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format and lint Python with Ruff using line length 120, target version 3.11, rules E/F/W/I/PL/UP, and single-line imports; avoid reformatting unrelated code.
Files:
src/aiq_agent/agents/chat_researcher/models/__init__.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/test_workflow_runner.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Register new data sources indata_source_registryso the UI can toggle them.
Respect authenticated data sources by honoringrequires_auth, per-user token pass-through, backend token validators, and owner guardrails before loading protected report or artifact context.
Do not weaken or bypassAuthMiddleware, token validators, or authentication gating without prior design discussion.
Files:
src/aiq_agent/agents/chat_researcher/models/__init__.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not edit adjacent repositories; keep changes inside this repository and scope changes insources/*to the smallest affected package.
Run the narrowest relevant validation command first and broaden testing only when changes cross shared boundaries.
For substantial behavior, authentication, UI, or architecture changes, open a design discussion before coding.
Keep pull requests scoped, avoid unrelated files and generated artifacts, do not include secrets, and provide validation commands and results.
Every commit must use DCO sign-off viagit commit -sand contain a validSigned-off-bytrailer.
Treatsources/*as independent packages and prefer the smallest change scoped to the package being modified.Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts.
Files:
src/aiq_agent/agents/chat_researcher/models/__init__.pymcp/REFERENCE_PARITY.mdtests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/test_workflow_runner.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,ts,tsx,js,jsx}: Never commit secrets, tokens, or environment-specific hostnames; use environment variables andSecretStr, and resolve API keys at runtime.
Never print or log secret values, including in tool output or error messages.
Missing-secret paths must degrade gracefully by stubbing or skipping rather than crashing or leaking information.
Files:
src/aiq_agent/agents/chat_researcher/models/__init__.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/test_workflow_runner.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,pyi}: Runuv run ruff check .anduv run ruff format --check .for Python changes.
Runuv run pytestand add or update tests for Python behavior changes.
Files:
src/aiq_agent/agents/chat_researcher/models/__init__.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/test_workflow_runner.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/models/__init__.pysrc/aiq_agent/agents/chat_researcher/models/result.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.py
mcp/**/*.{py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For MCP changes, run the MCP project development dependency setup and
uv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pymcp/tests/test_workflow_runner.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Open an issue or discussion before large design changes, public APIs, deployment changes, or contributor workflow changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Target the `develop` branch unless a maintainer requests a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Make the smallest coherent change and add or update tests for behavior changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Sign off every commit with `git commit -s`; commits without sign-off may be rejected.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Run the narrowest local validation command that covers the change and include exact output or a workflow link in the pull request.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Open pull requests into `develop`, complete the PR template with exact validation evidence, and address feedback until required checks and code-owner review pass.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:37.061Z
Learning: Search existing issues and pull requests before opening new work.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:42.163Z
Learning: Use the repository's development and quality workflow: prefer `./scripts/dev.sh` commands or run `pre-commit run --all-files`, `pytest`, and the configured formatters directly.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:52.561Z
Learning: Use the documented `uv` commands to install dependencies, run the full test suite, run coverage, or run targeted tests; the MCP project uses its own environment and lockfile.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:52.561Z
Learning: Refer to each benchmark's README for benchmark-specific instructions and consult the Customization guide when adding evaluation harnesses.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:25:52.561Z
Learning: For debugging, enable verbose CLI logging or workflow configuration, use Phoenix tracing when configured, and consult the documented troubleshooting steps for import, authentication, tool-discovery, and pre-commit issues.
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/chat_researcher/test_agent.py
🔇 Additional comments (14)
src/aiq_agent/agents/chat_researcher/models/__init__.py (1)
20-38: LGTM!src/aiq_agent/agents/chat_researcher/models/result.py (1)
18-49: LGTM!src/aiq_agent/agents/chat_researcher/models/state.py (1)
30-30: LGTM!Also applies to: 62-63, 81-81
src/aiq_agent/agents/chat_researcher/agent.py (1)
55-58: LGTM!Also applies to: 72-73, 172-172, 237-237, 250-250, 263-263, 302-305, 364-391
src/aiq_agent/agents/chat_researcher/register.py (1)
34-34: LGTM!Also applies to: 43-52, 69-98, 570-570, 588-610, 654-669, 681-684, 704-704, 717-717
tests/aiq_agent/agents/chat_researcher/test_register_helpers.py (1)
18-19: LGTM!Also applies to: 26-28, 31-44, 46-70
mcp/tests/test_workflow_runner.py (1)
1-31: LGTM!Also applies to: 93-160
mcp/tests/test_postgres_job_store.py (1)
22-22: LGTM!Also applies to: 30-54
mcp/REFERENCE_PARITY.md (1)
69-73: LGTM!tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
25-29: LGTM!Also applies to: 214-239, 584-585
mcp/tests/test_jobs.py (1)
19-24: LGTM!Also applies to: 30-47, 59-67, 639-664, 766-798, 994-1016
mcp/tests/test_client_session_integration.py (1)
34-36: LGTM!Also applies to: 74-88, 447-508
mcp/src/aiq_mcp/jobs.py (1)
27-27: LGTM!Also applies to: 236-267, 284-292
mcp/src/aiq_mcp/workflow_runner.py (1)
109-132: 🩺 Stability & AvailabilityNo issue:
_render_workflow_response()turns a missingworkflow_outcomeintoWorkflowSuccess(result=...), sorun_query()cannot returnNoneon the success path.> Likely an incorrect or invalid review comment.
|
/ok to test 77144a4 |
Signed-off-by: Tanner Leach <tleach@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
214-239: 📐 Maintainability & Code Quality | 🔵 TrivialRun the required Python validation before merge.
Run the narrowest relevant tests first, then
uv run ruff check .,uv run ruff format --check ., anduv run pytest; include the exact results in the PR.As per coding guidelines, Python changes require Ruff and pytest validation. Based on learnings, run the narrowest relevant suite first and include the exact output in the PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/aiq_agent/agents/chat_researcher/test_agent.py` around lines 214 - 239, Run the focused tests covering test_shallow_research_failure_sets_terminal_outcome first, then execute uv run ruff check ., uv run ruff format --check ., and uv run pytest; report the exact results from each command in the PR.Sources: Coding guidelines, Learnings
src/aiq_agent/agents/chat_researcher/agent.py (1)
570-580: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReset checkpoint state for dict turns in
run().
Atsrc/aiq_agent/agents/chat_researcher/agent.py:570-580, the dict path bypasses theshallow_result/workflow_outcomereset, so a follow-up turn on the same thread can inherit a priorWorkflowFailure. Normalize the dict branch too and add a regression test that callsrun()with a dict after a failing turn.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiq_agent/agents/chat_researcher/agent.py` around lines 570 - 580, Update the dict-state branch in run() to normalize turn-boundary checkpoint fields like shallow_result and workflow_outcome to None, matching the non-dict branch and preventing prior failures from carrying into follow-up turns. Add a regression test that runs a failing turn, then calls run() with a dict on the same thread and verifies the stale failure state is cleared.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 410-411: Remove the raw active_report_job_id from the timeout
warning in the report ask exception path; use log_identifier_ref for a redacted
identifier or log only the timeout error type. Extend redaction coverage to the
agent logger so this path verifies the bearer capability is never emitted.
---
Outside diff comments:
In `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 570-580: Update the dict-state branch in run() to normalize
turn-boundary checkpoint fields like shallow_result and workflow_outcome to
None, matching the non-dict branch and preventing prior failures from carrying
into follow-up turns. Add a regression test that runs a failing turn, then calls
run() with a dict on the same thread and verifies the stale failure state is
cleared.
In `@tests/aiq_agent/agents/chat_researcher/test_agent.py`:
- Around line 214-239: Run the focused tests covering
test_shallow_research_failure_sets_terminal_outcome first, then execute uv run
ruff check ., uv run ruff format --check ., and uv run pytest; report the exact
results from each command in the PR.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 07fb75c6-36f1-44b2-a819-76cee67d972f
📒 Files selected for processing (4)
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Pytest and Coverage
🧰 Additional context used
📓 Path-based instructions (6)
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Ruff for Python linting and formatting with line length 120, target Python 3.11, rules E, F, W, I, PL, UP, and single-line imports via isort
force-single-line. Do not reformat unrelated code.
Files:
tests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/agents/chat_researcher/test_agent.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
New tools and data sources must be implemented as NeMo Agent Toolkit functions registered with
@register_function; their config schemas must inherit fromFunctionBaseConfig.For Python repository changes, run
uv run ruff check .,uv run ruff format --check ., anduv run pytestas applicable.
Files:
tests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,pyi}: Never commit secrets or tokens; use environment variables andSecretStr, and resolve API keys at runtime.
Never print or log secret values, including in tool output or error messages.
Missing-secret paths must degrade gracefully by stubbing or skipping rather than crashing or leaking secrets.
Respect authenticated data sources by honoringrequires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent.
Do not weaken or bypassAuthMiddleware, validators, or authentication gating without prior design discussion.
Files:
tests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Stay inside this repository and do not edit adjacent repositories. Treat eachsources/*package as independent and prefer the smallest package-scoped change.
Run the narrowest relevant validation command first, broadening to the full suite only when a change crosses shared boundaries.
For substantial behavior, authentication, UI, or architecture changes, open a design discussion before coding rather than landing a large unreviewed change.
Keep pull requests scoped: avoid unrelated files, accidental generated artifacts, and secrets, and provide validation evidence with commands and results.
Every commit must be signed off withgit commit -s; commits must contain aSigned-off-bytrailer.
**/*: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts in contributions.
Target thedevelopbranch for pull requests unless a maintainer requests a release branch.
Add or update tests for behavior changes.
Files:
tests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Ruff for Python linting and formatting with line length 120, target Python 3.11, rules E, F, W, I, PL, UP, and single-line imports via isort
force-single-line. Do not reformat unrelated code.
Files:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:15.613Z
Learning: Sign off every commit with `git commit -s`; commits without sign-off may be rejected.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:15.613Z
Learning: Run the narrowest relevant local validation before opening a pull request and include the exact output or workflow link in the PR.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:15.613Z
Learning: Open an issue or discussion before large design changes, public APIs, deployment changes, or contributor workflow changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:15.613Z
Learning: Use the maintainer-reviewed pull-request workflow and address feedback until required checks and code-owner review pass.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:29.160Z
Learning: Use the documented `uv` commands to synchronize environments and run the main project, coverage, targeted, or MCP test suites.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:29.160Z
Learning: Refer to each benchmark's README for benchmark-specific testing details.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T19:58:29.160Z
Learning: Use verbose CLI logging, Phoenix tracing, and the documented troubleshooting steps when debugging.
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/chat_researcher/test_agent.py
🔇 Additional comments (4)
tests/aiq_agent/agents/chat_researcher/test_register_helpers.py (1)
18-68: LGTM!Also applies to: 137-152
tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
25-29: LGTM!Also applies to: 584-594, 607-607, 625-627, 668-668
src/aiq_agent/agents/chat_researcher/agent.py (1)
55-58: LGTM!Also applies to: 72-75, 172-172, 237-243, 248-255, 261-269, 302-305, 364-391, 404-407, 412-415, 427-428, 443-446, 457-465
src/aiq_agent/agents/chat_researcher/register.py (1)
34-34: LGTM!Also applies to: 43-52, 64-98, 139-139, 570-570, 588-610, 654-669, 681-684, 704-704, 717-717
Signed-off-by: Tanner Leach <tleach@nvidia.com>
The per-user MCP tools block imported PerUserMcpSourceUnavailableError inside the same try that guards the aiq_api imports. In the standalone MCP profile (which intentionally ships without aiq_api), the first aiq_api import raises ModuleNotFoundError, and then evaluating `except PerUserMcpSourceUnavailableError` raises UnboundLocalError, bypassing the broad `except Exception` that was meant to continue. Result: every shallow query on the standalone MCP server failed with "Research could not be completed. Please try again." Resolve the reconnect exception type up front (guarded) and treat a missing aiq_api as an expected skip, so shallow research proceeds with its base tools. The per-user reconnect message is preserved when aiq_api is present. Add register-level regression tests covering both the aiq_api-absent skip path (previously crashed with UnboundLocalError) and the reconnect path. Verified live against a standalone MCP container: a shallow query that returned state="failed" now returns state="complete" with cited results. Fixes: AIQ-002 Signed-off-by: Tanner Leach <tleach@nvidia.com>
Document source checkout and release container support, mark the locally buildable MCP wheel as private, and cover the classifier in packaging tests. Signed-off-by: Tanner Leach <tleach@nvidia.com>
The report job UUID is an opaque bearer capability in the MCP security model: all callers share the anonymous principal, so anyone holding the UUID can read or edit that report. Three report follow-up log sites in the chat researcher logged it verbatim (ask timeout, ask failure, and edit-submission failure), leaking the capability into logs. Route the identifier through the existing log_identifier_ref helper so only a stable sha256:<12hex> correlation ref is logged, matching the convention already used in register.py and the MCP job runner. A small None-safe _report_ref wrapper handles the inline path where active_report_job_id is unset. Add caplog coverage on all three sites asserting the raw UUID is absent and the redacted ref is present. Source: PR NVIDIA-AI-Blueprints#319 review (discussion r3582250506). Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
The CI postgres service creates a database named aiq_mcp_tests (plural), but require_test_database_url only accepted the _test suffix, causing all PostgreSQL-backed tests to error at setup with a ValueError. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
The MCP job path classified every research query twice: JobManager.submit() classified to persist depth and choose the poll cadence, then the chat graph re-classified at its intent_classifier entry point. At temperature 0.5 the second decision could diverge from the first, so the public job depth / estimated_duration / poll cadence could describe different work than actually ran, and every submit paid intent-classifier latency and cost twice. Make JobManager the single owner of the decision. submit()'s depth is threaded through _run_job -> run_query into a request-scoped ContextVar (chat_researcher/preclassification.py); the intent_classifier node reuses it and returns without calling the LLM. The hook is strictly opt-in: API/CLI callers never set it, user_intent/depth_decision stay unset, and the short-circuit never fires for them. Adds a unit test (node reuses the preset, no LLM call), an MCP consistency test (classify called once; persisted depth == executed run depth), and a real-workflow integration test that drives the actual intent_classifier through nat.Function.ainvoke and proves the preset overrides a divergent LLM decision with no second classification. Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
/ok to test 3be0337 |
WorkflowRunner._close_owned_checkpointers() reaches into aiq_agent.common's
private _checkpointers / _postgres_pools dicts to close the handles it created,
reading them via getattr(..., {}). If aiq_agent renamed or removed those private
names, cleanup silently became a no-op and the connections/pools leaked.
Resolve the caches up front and warn (naming the missing cache) when they are
absent, instead of skipping silently. The check runs before the empty-owned-keys
guard so a rename -- which leaves start()'s snapshot empty and would otherwise
bypass detection -- is still surfaced. There is no public cleanup API in
aiq_agent.common to reuse, so warn-only is the intended fallback.
Adds tests: warns when the caches are missing (with and without recorded owned
keys) and stays silent when caches are present but nothing is owned.
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Validate supported-client endpoints before constructing the HTTP client so credential-bearing, query-token, and fragment URLs cannot be requested or echoed. Cover accepted and rejected forms and document the constraint. Fixes: AIQ-007 Signed-off-by: Tanner Leach <tleach@nvidia.com>
The UPDATE+fallback pattern in JobStore.record_poll() fell back to the unscoped get() when the principal-scoped UPDATE touched zero rows (job terminal or nonexistent). get() has no AND principal filter, so a caller holding any job UUID could observe a terminal job owned by a different principal. Replace the get() fallback with an inline SELECT that includes AND principal = \$2, maintaining the same isolation contract as the UPDATE. get() is intentionally left unscoped — it is an internal/runner path with no principal-enforcement callers. Also tighten the existing wrong-principal active-job assertion in the reconciliation semantics test (record_poll with the wrong principal now returns None rather than the unguarded row) and add a regression test for the terminal-state cross-principal case. Signed-off-by: Tanner Leach <tleach@nvidia.com>
The report-rewriter child job id is the same bearer-capability class as the chat-researcher report job UUID: in the MCP security model all callers share the anonymous principal, so anyone holding the UUID can read or edit that report. The success-path completion log in report_rewriter/agent.py logged it verbatim on every completed rewrite, leaking the capability into logs. Route the identifier through the existing log_identifier_ref helper via a None-safe _job_ref wrapper, so only a stable sha256:<12hex> correlation ref is logged (falling back to "none" when job_id is unset), matching the redaction already applied to the chat-researcher report follow-up log sites. Adds caplog coverage asserting the raw UUID is absent and the redacted ref is present on the success path, plus a None-job-id case; both proven to fail against the pre-fix log line. Source: PR NVIDIA-AI-Blueprints#319 review (AIQ issue 014, follow-up to issue 003). Signed-off-by: Tanner Leach <tleach@nvidia.com>
Add focused tests for validate_lock_sources in mcp/scripts/check_license_inventory.py, which enforces the MCP public-source contract (public PyPI plus the approved editable path sources). Covers acceptance of the exact approved contract, rejection of unapproved registries and non-editable or malformed lock sources, local source-set mismatches, dependency-name canonicalization, and a regression guard that the committed mcp/uv.lock satisfies the contract. Pins the current exact-match editable-path behavior as characterization. Resolves AIQ-011 (PR NVIDIA-AI-Blueprints#319 thread r3580087766). Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
/ok to test d90bc8a |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/aiq_agent/agents/chat_researcher/agent.py (1)
581-583: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReset transient outcomes for dictionary inputs.
Dictionary callers bypass the per-turn reset at Lines 590-591. A checkpointed prior
WorkflowFailurecan therefore survive into a successful turn and be persisted as a failed MCP job. Copy the mapping and clear both transient fields.Proposed fix
if isinstance(state, dict): - input_state = state + input_state = { + **state, + "shallow_result": None, + "workflow_outcome": None, + } messages = state.get("messages", [])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiq_agent/agents/chat_researcher/agent.py` around lines 581 - 583, Update the dictionary-input branch in the agent state handling to copy the incoming mapping rather than mutate or reuse it directly, then clear both transient outcome fields before processing the turn. Preserve the existing messages extraction while ensuring prior WorkflowFailure state cannot be carried into persistence after a successful turn..github/workflows/ci.yml (1)
230-236: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRun the import canary in the production MCP environment.
This invokes the dependency script without
--verify-imports, so the--no-dev --no-editableclosure can still pass while imports or entry points fail at runtime. Add the flag to Line 236 and retain it in CI.As per path instructions, automation changes require reproducible uv setup and consistency with the documented validation matrix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 230 - 236, Update the production MCP environment validation in the workflow step named “Create MCP production environment” to invoke mcp/scripts/check_runtime_dependencies.py with the --verify-imports flag, preserving the existing frozen, no-dev, no-default-groups, and no-editable uv setup.Source: Path instructions
mcp/src/aiq_mcp/workflow_runner.py (1)
77-90: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact checkpointer keys in cleanup logs.
keyis the job-backedconversation_id; Lines 85 and 90 log it verbatim, bypassing the redaction already used byrun_query. Uselog_identifier_ref(key)in both messages.Proposed fix
checkpointers, postgres_pools = caches for key in list(self._owned_checkpointer_keys): + key_ref = log_identifier_ref(key) checkpointer = checkpointers.pop(key, None) ... - logger.debug("Closed SQLite checkpointer connection: %s", key) + logger.debug("Closed SQLite checkpointer connection: %s", key_ref) ... - logger.debug("Closed Postgres checkpointer pool: %s", key) + logger.debug("Closed Postgres checkpointer pool: %s", key_ref)Based on PR objectives, job UUIDs are bearer capabilities. As per coding guidelines, “Never print or log secret values, including through tool output or error messages.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/src/aiq_mcp/workflow_runner.py` around lines 77 - 90, Update the cleanup debug logs in the checkpointer cleanup loop to redact the job-backed key before logging it. Specifically, in the messages associated with closing SQLite connections and PostgreSQL pools, replace the raw key formatting with the existing log_identifier_ref(key) helper, matching the redaction used by run_query.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/deployment/docker-build.md`:
- Around line 67-69: Update the deployment documentation’s MCP sync command to
explicitly include the --frozen flag before --project /app/mcp, preserving the
existing explanation that the build uses mcp/uv.lock.
In `@mcp/src/aiq_mcp/job_store.py`:
- Around line 307-333: Update the cleanup query in the job-store sweep to select
only terminal jobs before deleting checkpoint state; do not clean up expired
jobs whose workflow is still running. Preserve the existing locked, batched
deletion flow for terminal jobs and add an integration test proving active
workflows can still write checkpoints without their state being deleted.
In `@src/aiq_agent/agents/shallow_researcher/register.py`:
- Around line 150-163: Separate the top-level aiq_api import from the
open_per_user_mcp_tools call in the shallow researcher setup. Only treat a
ModuleNotFoundError proving aiq_api itself is unavailable as the
standalone-profile fallback; let ModuleNotFoundError or other failures from
open_per_user_mcp_tools follow the existing reconnect/error handling, preserving
requires_auth and authentication gating. Add a regression test covering
open_per_user_mcp_tools raising ModuleNotFoundError.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 230-236: Update the production MCP environment validation in the
workflow step named “Create MCP production environment” to invoke
mcp/scripts/check_runtime_dependencies.py with the --verify-imports flag,
preserving the existing frozen, no-dev, no-default-groups, and no-editable uv
setup.
In `@mcp/src/aiq_mcp/workflow_runner.py`:
- Around line 77-90: Update the cleanup debug logs in the checkpointer cleanup
loop to redact the job-backed key before logging it. Specifically, in the
messages associated with closing SQLite connections and PostgreSQL pools,
replace the raw key formatting with the existing log_identifier_ref(key) helper,
matching the redaction used by run_query.
In `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 581-583: Update the dictionary-input branch in the agent state
handling to copy the incoming mapping rather than mutate or reuse it directly,
then clear both transient outcome fields before processing the turn. Preserve
the existing messages extraction while ensuring prior WorkflowFailure state
cannot be carried into persistence after a successful turn.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b32ba7ee-36d3-4551-bb3f-f5ce3765489d
⛔ Files ignored due to path filters (1)
mcp/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/workflows/ci.ymlREADME.mddocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.mdmcp/README.mdmcp/SECURITY.mdmcp/pyproject.tomlmcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/db_url.pymcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_db_url.pymcp/tests/test_deployment_assets.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_preclassification_integration.pymcp/tests/test_release_checks.pymcp/tests/test_workflow_runner.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/preclassification.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/shallow_researcher/register.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Format and lint Python with Ruff using line length 120, target Python 3.11, rule sets E, F, W, I, PL, UP, and single-line imports; avoid reformatting unrelated code.
Never print or log secret values, including through tool output or error messages.
Missing-secret paths must degrade gracefully by stubbing or skipping rather than crashing or leaking values.For Python changes, run
uv run ruff check .,uv run ruff format --check ., anduv run pytestas applicable.
Files:
src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/preclassification.pymcp/tests/test_preclassification_integration.pymcp/src/aiq_mcp/db_url.pytests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pymcp/tests/test_db_url.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.pymcp/tests/test_workflow_runner.pymcp/tests/test_config_and_packaging.pymcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_deployment_assets.pymcp/tests/test_release_checks.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_postgres_job_store.py
**/*.{py,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Register every new data source in
data_source_registryso the UI can toggle it.
Files:
src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/preclassification.pymcp/tests/test_preclassification_integration.pymcp/src/aiq_mcp/db_url.pytests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pymcp/tests/test_db_url.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.pymcp/tests/test_workflow_runner.pymcp/tests/test_config_and_packaging.pymcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_deployment_assets.pymcp/tests/test_release_checks.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_postgres_job_store.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Respect authenticated data sources by honoringrequires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent.
Do not weaken or bypassAuthMiddleware, validators, or authentication gating without prior design discussion.
Files:
src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/preclassification.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Stay within this repository and do not edit adjacent repositories; treat eachsources/*package as independent and prefer the smallest package-scoped change.
Run the narrowest relevant validation command first and broaden to the full suite only when a change crosses shared boundaries.
Update documentation and seek a design discussion before coding substantial behavior, authentication, UI, or architecture changes.
Keep pull requests scoped: avoid unrelated files, generated artifacts, and secrets, and provide validation commands and results.
Every commit must use DCO sign-off viagit commit -sand contain aSigned-off-bytrailer.
Load the relevant task-specific maintainer skill from.agents/skills/before starting a workflow it covers; do not treat API-consumer skills inskills/as an in-product runtime.Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts.
Files:
src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/preclassification.pymcp/tests/test_preclassification_integration.pymcp/src/aiq_mcp/db_url.pyREADME.mdmcp/README.mdtests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pymcp/tests/test_db_url.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.pymcp/SECURITY.mdmcp/tests/test_workflow_runner.pymcp/tests/test_config_and_packaging.pydocs/source/deployment/docker-build.mdmcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/agents/shallow_researcher/register.pydocs/source/integration/mcp-server.mdmcp/pyproject.tomlsrc/aiq_agent/agents/chat_researcher/agent.pymcp/tests/test_deployment_assets.pymcp/tests/test_release_checks.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_postgres_job_store.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/preclassification.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.py
mcp/**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For changes under
mcp/, run the MCP project's development dependency setup anduv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/tests/test_preclassification_integration.pymcp/src/aiq_mcp/db_url.pymcp/tests/test_db_url.pymcp/tests/test_workflow_runner.pymcp/tests/test_config_and_packaging.pymcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/test_deployment_assets.pymcp/tests/test_release_checks.pymcp/tests/test_jobs.pymcp/tests/test_client_session_integration.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_postgres_job_store.py
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
README.mddocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.md
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update documentation under
docs/source/when behavior, configuration, or workflows change; keep documentation canonical rather than duplicating full pages in skills.
Files:
docs/source/deployment/docker-build.mddocs/source/integration/mcp-server.md
**/*.{toml,lock}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Dependency changes must leave both project locks current; validate with
uv lock --checkanduv lock --project mcp --check.
Files:
mcp/pyproject.toml
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
mcp/pyproject.toml.github/workflows/ci.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:10.011Z
Learning: Add or update tests for behavior changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:10.011Z
Learning: Sign off every commit with `git commit -s`; commits without sign-off may be rejected.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:10.011Z
Learning: Open an issue or discussion before large design changes, public APIs, deployment changes, or contributor workflow changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:14.203Z
Learning: When the helper script is unavailable, run `pre-commit run --all-files`, `pytest`, and the configured formatters directly.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:14.203Z
Learning: Run `pre-commit run --all-files` to execute repository-wide checks, including Ruff, lock checks, secret detection, notebook output clearing, and Markdown link validation.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:19.774Z
Learning: Create focused branches from `develop` for changes, add or update tests, and open pull requests against `develop` unless targeting a release branch is explicitly requested by a maintainer.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:19.774Z
Learning: Run the narrowest relevant local validation checks for each change and record the exact commands in the pull request description.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:19.774Z
Learning: For general project changes, run `uv sync --group dev`, `uv run ruff check .`, `uv run ruff format --check .`, and `uv run pytest`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:19.774Z
Learning: All contributors must sign off their commits using the Developer's Certificate of Origin, for example with `git commit -s -m "Your message"`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-15T10:58:19.774Z
Learning: Contributors certify that they have the right to submit their contribution under the applicable open-source license and acknowledge that contribution records, including sign-off personal information, are public and maintained indefinitely.
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/agents/chat_researcher/test_agent.py
🪛 ast-grep (0.44.1)
mcp/tests/test_deployment_assets.py
[warning] 176-176: Do not make http calls without encryption
Context: "http://[::1]:9001/mcp"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 176-176: Do not make http calls without encryption
Context: "http://[::1]:9001/health"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 192-192: Do not make http calls without encryption
Context: "http://alice@example.com/mcp"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[error] 250-255: Command coming from incoming request
Context: subprocess.run(
[sys.executable, str(_SMOKE_SCRIPT), "--url", endpoint, "--health-timeout", "0.01"],
check=False,
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 OpenGrep (1.25.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 324-327: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 329-332: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (27)
src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.py (1)
37-37: LGTM!Also applies to: 110-120
src/aiq_agent/agents/report_rewriter/agent.py (1)
28-44: LGTM!Also applies to: 193-193
src/aiq_agent/agents/chat_researcher/preclassification.py (1)
31-57: LGTM!mcp/tests/test_preclassification_integration.py (1)
33-102: LGTM!tests/aiq_agent/agents/report_rewriter/test_agent.py (1)
7-15: LGTM!Also applies to: 207-254
tests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.py (1)
29-29: LGTM!Also applies to: 499-533
tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
18-18: LGTM!Also applies to: 31-31, 219-241, 586-729
src/aiq_agent/agents/chat_researcher/agent.py (1)
49-74: LGTM!Also applies to: 183-183, 248-248, 261-261, 274-274, 313-316, 375-476
mcp/src/aiq_mcp/db_url.py (1)
9-10: LGTM!Also applies to: 25-35
README.md (1)
278-278: LGTM!Also applies to: 340-360
mcp/README.md (1)
15-24: LGTM!Also applies to: 90-100, 123-129, 159-164
mcp/tests/test_db_url.py (1)
32-59: LGTM!mcp/src/aiq_mcp/job_store.py (1)
76-82: LGTM!Also applies to: 280-293
mcp/tests/test_checkpoint_todos.py (1)
36-36: LGTM!Also applies to: 421-467
mcp/tests/test_postgres_job_store.py (1)
24-24: LGTM!Also applies to: 51-55, 264-325, 690-810
.github/workflows/ci.yml (1)
267-324: LGTM!mcp/tests/test_workflow_runner.py (1)
196-249: LGTM!mcp/scripts/protocol_smoke.py (1)
33-34: LGTM!Also applies to: 58-65
mcp/tests/test_deployment_assets.py (1)
163-267: LGTM!mcp/SECURITY.md (1)
93-96: LGTM!Also applies to: 126-135
mcp/tests/test_config_and_packaging.py (1)
128-128: LGTM!docs/source/integration/mcp-server.md (1)
39-47: LGTM!Also applies to: 259-260
mcp/pyproject.toml (1)
20-20: LGTM!Also applies to: 59-59
mcp/tests/test_release_checks.py (1)
24-25: LGTM!Also applies to: 243-358
mcp/tests/test_jobs.py (1)
48-53: LGTM!Also applies to: 62-64, 244-277, 681-682
mcp/tests/test_client_session_integration.py (1)
38-38: LGTM!Also applies to: 75-75, 84-84, 581-581, 593-593
mcp/src/aiq_mcp/jobs.py (1)
179-179: LGTM!Also applies to: 236-236, 249-251
| That builder syncs the frozen MCP project (`uv sync --project /app/mcp`) from | ||
| `mcp/uv.lock`, independently of the root image. The root image stays within | ||
| NAT's `cryptography<47` constraint; the audited MCP container profile pins |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Show the actual frozen sync command.
The parenthesized command omits --frozen while describing a frozen build. Document uv sync --frozen --project /app/mcp so readers do not reproduce a lock-mutating build path.
Proposed fix
-That builder syncs the frozen MCP project (`uv sync --project /app/mcp`) from
+That builder syncs the frozen MCP project (`uv sync --frozen --project /app/mcp`) from📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| That builder syncs the frozen MCP project (`uv sync --project /app/mcp`) from | |
| `mcp/uv.lock`, independently of the root image. The root image stays within | |
| NAT's `cryptography<47` constraint; the audited MCP container profile pins | |
| That builder syncs the frozen MCP project (`uv sync --frozen --project /app/mcp`) from | |
| `mcp/uv.lock`, independently of the root image. The root image stays within | |
| NAT's `cryptography<47` constraint; the audited MCP container profile pins |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/deployment/docker-build.md` around lines 67 - 69, Update the
deployment documentation’s MCP sync command to explicitly include the --frozen
flag before --project /app/mcp, preserving the existing explanation that the
build uses mcp/uv.lock.
Source: Path instructions
| except ImportError: | ||
| # Standalone MCP profile without aiq_api: expected — continue with base tools. | ||
| logger.debug("aiq_api unavailable; skipping per-user MCP tools for shallow research") | ||
| except Exception as exc: | ||
| if PerUserMcpSourceUnavailableError is not None and isinstance( | ||
| exc, PerUserMcpSourceUnavailableError | ||
| ): | ||
| # The user explicitly selected a protected source we can't resolve | ||
| # (e.g. token expired). Surface a reconnect message instead of | ||
| # silently answering without it. | ||
| from langchain_core.messages import AIMessage | ||
|
|
||
| return ShallowResearchAgentState(messages=state.messages + [AIMessage(content=str(exc))]) | ||
| logger.exception("Failed to resolve per-user MCP tools for shallow research; continuing") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Only skip when aiq_api itself is absent.
This try also awaits open_per_user_mcp_tools, so an ImportError from a selected connector or auth dependency is treated as the standalone profile and the request proceeds without its selected protected source. Separate imports from tool resolution; only downgrade ModuleNotFoundError for top-level aiq_api, and surface later failures as reconnect/error states. Add a regression test where open_per_user_mcp_tools raises ModuleNotFoundError.
As per coding guidelines, authenticated data sources must honor requires_auth and authentication gating.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/aiq_agent/agents/shallow_researcher/register.py` around lines 150 - 163,
Separate the top-level aiq_api import from the open_per_user_mcp_tools call in
the shallow researcher setup. Only treat a ModuleNotFoundError proving aiq_api
itself is unavailable as the standalone-profile fallback; let
ModuleNotFoundError or other failures from open_per_user_mcp_tools follow the
existing reconnect/error handling, preserving requires_auth and authentication
gating. Add a regression test covering open_per_user_mcp_tools raising
ModuleNotFoundError.
Source: Coding guidelines
Signed-off-by: Tanner Leach <tleach@nvidia.com>
2 participants
Overview
Add a standalone, public AI-Q MCP server under
mcp/without the MaaS/MOS SDK. The server exposes exactly three stateless Streamable HTTP tools—submit_query,poll_query, andget_final_report—and keeps asynchronous research jobs in PostgreSQL while reusing the existing AI-Q/NAT workflow.This change also adds the public NIM + Tavily workflow config, container/Compose assets, user and security documentation, compatibility tests, release smoke tests, dependency/license evidence, and protected CI coverage.
Key scope and security boundaries:
anonymousprincipal, and each random job UUID is a bearer capability.AIQ_MCP_ALLOWED_HOSTS,AIQ_MCP_ALLOWED_ORIGINS, and CORS are DNS-rebinding/browser safeguards; they are not identity or authorization controls.The frozen compatibility contract and intentional public adaptations are documented in
mcp/REFERENCE_PARITY.md. Tool schemas, state transitions, polling cadence, PostgreSQL persistence, background-task ownership, todo normalization, and stable client error shapes remain compatible. The intentional adaptations are FastMCP transport/lifecycle ownership, anonymous capability access, public configuration, capability-safe logging, and deployment-owned TLS.Release-review follow-ups are explicit in
mcp/SECURITY.md:manual_review_requiredfor the releasing organization's legal/NOTICE review;Validation
uv run pre-commit run --all-files— all hooks passed, including Ruff, lock validation, secret detection, YAML checks, and Markdown link checks.PostgreSQL-backed MCP suite — 174 passed, 0 skipped, 96.67% coverage (90% gate).
Complete workspace suite — 1,542 passed, 8 skipped, 0 failed.
Frozen tool-contract extraction — exact ordered three-tool contract SHA-256
81eba67fadd56e64b58a84b700b202841f8636c93c6cbf63752507c8bf5ca96a.actionlint .github/workflows/ci.yml— passed with actionlint v1.7.12.Sphinx documentation build — 62 pages built; only three unrelated pre-existing missing-xref warnings.
Production-only
uv sync --frozen --package aiq-mcp-server --no-dev --no-default-groups --no-editableplusmcp/scripts/check_runtime_dependencies.py— passed.Unfiltered and exception-gated
uv audit, CycloneDX 1.5 SBOM generation, and exact license inventory — passed the documented engineering gates; evidence is archived byScript Validation.MCP wheel build — verified embedded Apache-2.0 license metadata.
Release Compose image boot — exact
/liveand/healthpayloads,GET /mcp405 contract, anonymous initialization, exact tool listing, and unknown-capability response passed throughmcp/scripts/protocol_smoke.py.I ran the relevant local checks or explained why they are not applicable.
I added or updated tests for behavior changes.
I updated documentation for user-facing or contributor-facing changes.
I confirmed this PR does not include secrets, credentials, or internal-only data.
I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with
git commit -sor an equivalent sign-off.Where should reviewers start?
docs/source/integration/mcp-server.mdfor the user-facing protocol, deployment, and security model.mcp/REFERENCE_PARITY.mdfor the compatibility boundary and intentional differences.mcp/src/aiq_mcp/server.pyandmcp/src/aiq_mcp/jobs.pyfor transport/lifecycle and state behavior..github/workflows/ci.ymlandmcp/SECURITY.mdfor mandatory test, SBOM, audit, license, and release-image gates.Related Issues
Summary by CodeRabbit
submit_query,poll_query,get_final_report, plusconfig_mcp.yml./live+/health, and protocol smoke testing.